On Thu, Dec 09, 2021 at 04:17:08PM +0000, Bruce Richardson wrote:

On Thu, Dec 09, 2021 at 08:15:01AM -0500, Aaron Conole wrote:

quoted

Jerin Jacob [off-list ref] writes:

quoted

On Thu, Dec 9, 2021 at 12:30 AM Jie Zhou [off-list ref] wrote:

quoted

Even though test_interrupts.c can compile on Windows, skip interrupt
tests for now since majority of eal_interrupt on Windows are stubs.
Will remove the skip after interrupt being fully enabled on Windows.

Signed-off-by: Jie Zhou <redacted>
Acked-by: Dmitry Kozlyuk <redacted>

---
 app/test/test_interrupts.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/app/test/test_interrupts.c b/app/test/test_interrupts.c
index 2a05399f96..eec9b2805b 100644
--- a/app/test/test_interrupts.c
+++ b/app/test/test_interrupts.c

@@ -12,6 +12,15 @@

 #include "test.h"

+#ifdef RTE_EXEC_ENV_WINDOWS

Across the series,
Instead of adding conditional compilation everywhere, Why not disable
specific file
for compilation for windows?
Purpose of EAL to abstract the differences in execution environment
and application
should not know that.

I think this was done because there would be two test lists in the meson
unit test file.  But this is the second comment about these ifdef's, and
maybe we should revisit that discussion.  Is there a different way to
accomplish not running the tests which are not appropriate for windows
builds, while not having two overlapping lists of unit tests in the
meson build file?

I'm wondering if a reasonable compromise solution might be to have the
build system expose a usable RTE_EXEC_ENV symbol that can be used in C-code
if statements rather than just in ifdefs. That would allow us to easily add
e.g.

if (RTE_EXEC_ENV == rte_env_linux)
    return TEST_SKIPPED;

into each test function needing it. Two lines of C-code is a lot easier to
add and manage than #ifdefs covering the whole file, or alternative lists
in meson.

Quick patch to allow C-code comparisons:

diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 1722924f67..b5b9fa14b4 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build

@@ -10,6 +10,12 @@ if not is_windows
     subdir('unix')
 endif
 
+exec_envs = {'freebsd': 0, 'linux': 1, 'windows': 2}
+foreach env, id:exec_envs
+    dpdk_conf.set('RTE_ENV_' + env.to_upper(), id)
+endforeach
+dpdk_conf.set('RTE_EXEC_ENV', exec_envs[exec_env])
+
 dpdk_conf.set('RTE_EXEC_ENV_' + exec_env.to_upper(), 1)
 subdir(exec_env)

A slightly simpler patch would just expose the environment as a string as
e.g. "linux", but I think numeric ids just make the code better rather than
having string comparisons. Alternatively, this could also be done via
C-code with ifdefs in EAL, but as it stands this meson change allows:

  if (RTE_EXEC_ENV == RTE_ENV_WINDOWS)
     ...

or:

  switch (RTE_EXEC_ENV) {
    case RTE_ENV_LINUX: ... ; break;
    case RTE_ENV_FREEBSD: ... ; break;
    case RTE_ENV_WINDOWS: ... ; break;
  }

Thoughts?

/Bruce