Repository navigation
[BUG] #5027 provides changes for logger blocking compilation with MSYS2 #6115
Description
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
Activity
lgirdwood commentedon Aug 10, 2022
@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
marc-hb commentedon Aug 15, 2022
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.pywhen run on Windows. Should be the fastest fix and very easy to revert in the very unlikely case someone wants to restore Windows compatibility.lgirdwood commentedon Aug 15, 2022
Intel has engineers who will want to do this, we need to work this out.
aborisovich commentedon Aug 16, 2022
@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'.
marc-hb commentedon Aug 17, 2022
OK, I'm quite surprised by this considering the earlier comments below. Change of mind?
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.abonislawski commentedon Aug 18, 2022
We are using it from the very beginning on windows with python tests just to parse input file
marc-hb commentedon Aug 18, 2022
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?
.ldcdictionary?aborisovich commentedon Aug 18, 2022
Yes, option sof-logger -d worked as far as I remember
abonislawski commentedon Aug 18, 2022
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).
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:
marc-hb commentedon Aug 18, 2022
OK, that must be why #5299 was called "resurrect" then :)
Thanks for the additional details,
sof-loggershould really be compiled in some Windows CI then, otherwise it will keep bitrotting and breaking regularly.I will try to
#ifdef linuxthe 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 commentedon Aug 18, 2022
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.
abonislawski commentedon Aug 19, 2022
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.
kfrydryx commentedon Aug 19, 2022
@marc-hb MSYS porting guide might be helpful, fe. there is a list of platform checks https://www.msys2.org/wiki/Porting/
lgirdwood commentedon Aug 19, 2022
@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.
aborisovich commentedon Aug 19, 2022
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-latestcontainer(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.marc-hb commentedon Aug 25, 2022
Fix submitted in #6191. Someone should try to compile #6191 with MSYS. I went for the
HAS_INOTIFYroute 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.aborisovich commentedon Aug 29, 2022
I'll test it in a moment
pjdobrowolski commentedon Aug 29, 2022
Checked and tested. Fix works.
aborisovich commentedon Aug 30, 2022
Yep, it works, sorry for late reply... Thanks @pjdobrowolski .