sof icon indicating copy to clipboard operation
sof copied to clipboard

[BUG] #5027 provides changes for logger blocking compilation with MSYS2

Open pjdobrowolski opened this issue 2 years ago • 15 comments

Changes made in #5027 require inotify which is available Linux Only. What blocks compilation.

Possible solution:

  • Switching off logger from default west compilation.
  • Revert commit and use different solution
  • Add ifdefs to turnoff that feature on Windows

pjdobrowolski avatar Aug 10 '22 12:08 pjdobrowolski

@pjdobrowolski can you ifdef atm for MSYS2 ? Sof logger is not the long term tool as Zephyr tooling will be used for log and trace. If you dont need to run it on MSYS2 then faster to remove from compilation if building on MSYS2

lgirdwood avatar Aug 10 '22 13:08 lgirdwood

Compiling sof-logger on Windows was always a long shot. Most of what the sof-logger code does is reading and parsing /sys/kernel/debug/sof/ which is never going to work on Windows. It can also decode a recorded trace off-line but who's going to capture on Linux and decode on Windows?

I recommend disabling sof-logger compilation in xtensa-build-zephyr.py when run on Windows. Should be the fastest fix and very easy to revert in the very unlikely case someone wants to restore Windows compatibility.

marc-hb avatar Aug 15 '22 08:08 marc-hb

but who's going to capture on Linux and decode on Windows?

Intel has engineers who will want to do this, we need to work this out.

lgirdwood avatar Aug 15 '22 12:08 lgirdwood

@marc-hb can you please fix the issue with logger by ifdefing this missing on MSYS2 header and feature? Reading debug-fs is not applicable to Windows anyway. I think it should be all 'ifdefed' somehow as 'linux only'.

aborisovich avatar Aug 16 '22 12:08 aborisovich

Intel has engineers who will want to do this, we need to work this out.

OK, I'm quite surprised by this considering the earlier comments below. Change of mind?

Sof logger is not the long term tool as Zephyr tooling will be used for log and trace. If you dont need to run it on MSYS2 then faster to remove from compilation if building on MSYS2

@marc-hb can you please fix the issue with logger by ifdefing this missing on MSYS2 header and feature?

I'd like to but I'm on sort-of-vacation right now and even when at work I don't have any Windows environment to test this on. Sorry to insist but I think disabling sof-logger compilation in the script is really the fastest, smallest and easy to revert short-term workaround until someone can provide the right #ifdef (any volunteer?) Again I don't think anyone has ever used this (soon deprecated) tool on Windows yet.

marc-hb avatar Aug 17 '22 17:08 marc-hb

Again I don't think anyone has ever used this (soon deprecated) tool on Windows yet.

We are using it from the very beginning on windows with python tests just to parse input file

abonislawski avatar Aug 18 '22 08:08 abonislawski

We are using it from the very beginning on windows with python tests just to parse input file

I didn't know sorry. This should really be in some CI then.

Not sure what you mean by "very beginning" but I remember @aborisovich struggling to compile it with MSYS2 6 months ago, see

  • #5299

What "input file" are you referring to? .ldc dictionary?

marc-hb avatar Aug 18 '22 11:08 marc-hb

We are using it from the very beginning on windows with python tests just to parse input file

I didn't know sorry. This should really be in some CI then.

Not sure what you mean by "very beginning" but I remember @aborisovich struggling to compile it with MSYS2 6 months ago, see

What "input file" are you referring to? .ldc dictionary?

Yes, option sof-logger -d worked as far as I remember image

aborisovich avatar Aug 18 '22 14:08 aborisovich

Not sure what you mean by "very beginning" but I remember @aborisovich struggling to compile it with MSYS2 6 months ago, see

sof-logger is much older than 5 months, in the past it was working fine with cygwin build, my first tries are from 2018 :) and it was used under windows to parse tgl, cnl and older platforms logs. On MTL we are using sof-logger too (but only as temporary debug solution).

What "input file" are you referring to? .ldc dictionary?

No, input file contains logs, substitute of debugfs trace file in linux. Python tests with diag driver can save dma output as .bin file and then we can parse it using sof-logger and .ldc dictionary. You dont need to work with linux debugfs, you can also use sof-logger as parser when all files provided, check sof-logger help:

-i infile -o outfile -l *.ldc_file Dump infile contents to outfile

abonislawski avatar Aug 18 '22 16:08 abonislawski

in the past it was working fine with cygwin build, my first tries are from 2018 :)

OK, that must be why #5299 was called "resurrect" then :)

Thanks for the additional details, sof-logger should really be compiled in some Windows CI then, otherwise it will keep bitrotting and breaking regularly.

I will try to #ifdef linux the inotify bits in the next few days[*], it should be easy enough but due to the Windows CI gap I will need someone else to actually test it on Windows (until it breaks again).

EDIT: I've been sick, sorry for the delay.

marc-hb avatar Aug 18 '22 20:08 marc-hb

will try to #ifdef linux the inotify bits in the next few days, it should be easy enough

Actually: what #ifdef "magic" should be used here? In other words, how does one make the difference between Linux versus Cygwin versus MSYS2? Considering the last two try hard to impersonate Linux on Windows...

This won't be blocking me but I'll need to know eventually.

marc-hb avatar Aug 18 '22 20:08 marc-hb

Best if can be solved inside but if not: maybe ifdef for this feature which depends on additional build parameter (in xtensa-build-zephyr.py there is check linux vs windows)? but this is not really an urgency, on our fpga platforms we have old builds from 2021 with cygwin1.dll :) So Im not sure if newest build with msys2 was ever used (maybe never will if this tool will be obsolete soon), there is rarely need to update if it just works.

Thats why I have no idea why it is building every time with each fw build, we should disable this anyway.

abonislawski avatar Aug 19 '22 06:08 abonislawski

Actually: what #ifdef "magic" should be used here?

@marc-hb MSYS porting guide might be helpful, fe. there is a list of platform checks https://www.msys2.org/wiki/Porting/

kfrydryx avatar Aug 19 '22 08:08 kfrydryx

@marc-hb @aborisovich @kfrydryx @abonislawski Is there a simple path to run basic WSL2 GH actions to prevent Windows failures ? I found https://github.com/features/actions which seems to indicate we can have a Windows container. However, I'm not a Windows user and no idea if this Windows container configuration would be suitable.

lgirdwood avatar Aug 19 '22 08:08 lgirdwood

Yes this can be done but somebody should start from Proof of Concept task and build locally project on Windows using new Zephyr SDK for Windows. I think nobody tried to do this. If this works we can easily add new processes for windows-latest container(I think it is Windows Server 2019) install there MSYS2. Then we can add MSYS and Zephyr sdk to cache https://github.com/marketplace/actions/cache so they are not reinstalled on every CI run. It looks like chocolatey package manager has MSYS2 package so it should be easy to install. Installing Zephyr SDK will be a bit harder probably but doable.

aborisovich avatar Aug 19 '22 12:08 aborisovich

Fix submitted in #6191. Someone should try to compile #6191 with MSYS. I went for the HAS_INOTIFY route which easily let me lie and pretend I didn't have inotify for local testing purposes so I'm pretty confident it should work on MSYS too.

marc-hb avatar Aug 25 '22 22:08 marc-hb

I'll test it in a moment

aborisovich avatar Aug 29 '22 09:08 aborisovich

Checked and tested. Fix works.

pjdobrowolski avatar Aug 29 '22 09:08 pjdobrowolski

Yep, it works, sorry for late reply... Thanks @pjdobrowolski .

aborisovich avatar Aug 30 '22 13:08 aborisovich