Hi,
06/03/2019 05:16, Anand Rawat:
> -# some libs depend on maths lib
> -add_project_link_arguments('-lm', language: 'c')
> -dpdk_extra_ldflags += '-lm'
> +if cc.find_library('lm', required : false).found()
> + # some libs depend on maths lib
> + add_project_link_arguments('-lm', language: 'c')
> + dpdk_extra_ldflags += '-lm'
> +endif
Either libmath is required or not.
I don't think it can be optional.
Why is it changed?
> # get binutils version for the workaround of Bug 97
> -ldver = run_command('ld', '-v').stdout().strip()
> -if ldver.contains('2.30')
> - if cc.has_argument('-mno-avx512f')
> - march_opt += '-mno-avx512f'
> - message('Binutils 2.30 detected, disabling AVX512 support as
> workaround for bug #97')
> +if host_machine.system() != 'windows'
> + ldver = run_command('ld', '-v').stdout().strip()
> + if ldver.contains('2.30')
> + if cc.has_argument('-mno-avx512f')
> + march_opt += '-mno-avx512f'
> + message('Binutils 2.30 detected, disabling AVX512
> support as workaround for bug #97')
> + endif
> endif
> endif
Same comment as in v1, you should check which linker is used.
This check is only for GNU ld 2.30.
> +if host_machine.system() != 'windows'
> + common_sources = files(
The definitive solution should be to compile all common EAL files.
Please explain what are the issues in the common files.
I think we should not remove them and fix them one by one.
You could provide a separate patch to skip some files for
making helloworld working.
> -subdir(join_paths('arch', arch_subdir))
> +
> +common_headers += files('include/rte_common.h')
> +if host_machine.system() != 'windows'
> + subdir(join_paths('arch', arch_subdir))
> +endif
Same comment as above, it should not be changed.
> -common_headers = files(
> +common_headers += files(
> 'include/rte_alarm.h',
> 'include/rte_branch_prediction.h',
> 'include/rte_bus.h',
> 'include/rte_bitmap.h',
> 'include/rte_class.h',
> - 'include/rte_common.h',
Why rte_common.h is moved first in the list?
> -deps += 'kvargs'
> +if host_machine.system() != 'windows'
> + deps += 'kvargs'
> +endif
Why kvargs is removed?
> --- /dev/null
> +++ b/lib/librte_eal/windows/eal/eal_lcore.c
> + /* Get the cpu core id value */
> +unsigned int
> +eal_cpu_core_id(unsigned int lcore_id)
> +{
> + return lcore_id;
> +}
For this function and the others in this file,
please add a comment explaining this is a stub, not the expected result.
You can add a TODO or FIXME tag.
> --- a/lib/meson.build
> +++ b/lib/meson.build
> +if host_machine.system() == 'windows'
> + libraries = ['eal'] # override libraries for windows
> +endif
Instead of simply "override", you could explain that the other libraries
are not yet supported on Windows.
> --- a/meson.build
> +++ b/meson.build
> -# build libs and drivers
> +# build libs
> subdir('lib')
> -subdir('buildtools')
> -subdir('drivers')
>
> -# build binaries and installable tools
> -subdir('usertools')
> -subdir('app')
> +if host_machine.system() != 'windows'
> + # build buildtools and drivers
> + subdir('buildtools')
> + subdir('drivers')
>
> -# build docs
> -subdir('doc')
> + # build binaries and installable tools
> + subdir('usertools')
> + subdir('app')
> + subdir('test')
> +
> + # build kernel modules if enabled
> + if get_option('enable_kmods')
> + subdir('kernel')
> + endif
> +
> + # build docs
> + subdir('doc')
> +endif
I don't like modifying this file.
Can we skip not supported directories inside the sub meson files?
