Open kiwichris opened 3 months ago
I have reviewed the CI failures and I do not think they are related to the changes I have made.
I just pushed a change to the master branch here which cleaned up some compiler warnings, and the subsequent CI build jobs for RTEMS 4.9 and 4.10 both succeeded. I also re-triggered those CI builds for this PR which unfortunately both failed again, so some code change inside this PR must have caused the failures.
I don't care about the pre-commit check failure, just the ability to build this module against RTEMS 4.9 and 4.10. Note that the EPICS Base 7.0 version that this CI is using at the moment doesn't include your RTEMS-6 PRs, and we expect future releases of this module to continue to build against older EPICS versions anyway — the CI jobs that build it for Linux against both 3.14 and 3.15 should hint at that.
@anjohnson I have resolved the issue with 4.10 and I assume 4.9. It seems the define _RTEMS_VIOLATE_KERNEL_VISIBILITY__
effects what is included by rtems.h
and in this case wkspace.h
was not being included. I have moved the include order to clean this up.
Unfortunately, when I try to compile (MVME6100 legacy RTEMS6) I get this: ... /include -I/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/StreamDevice/include -c ../devIocStatsWaveform.c In file included from ../os/RTEMS/devIocStatsOSD.h:84, from ../devIocStats.h:52, from ../devIocStatsWaveform.c:76: /home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/include/stdlib.h:260:21: error: macro "random" passed 1 arguments, but takes just 0 260 | long random (void); | ^ In file included from /home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/beatnik/lib/include/sys/malloc.h:39, from /home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/beatnik/lib/include/sys/mbuf.h:38, from ../os/RTEMS/devIocStatsOSD.h:65: /home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/beatnik/lib/include/rtems/rtems_bsdnet_internal.h:63: note: macro "random" defined here 63 | #define random() rtems_bsdnet_random() |
---|
make[2]: [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_BUILD:259: devIocStatsWaveform.o] Error 1 make[2]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats/O.RTEMS-beatnik' make[1]: [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_ARCHS:58: install.RTEMS-beatnik] Error 2 make[1]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats' make: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_DIRS:85: devIocStats.install] Error 2
e.g. in devIocStats/os/RTEMS/devIocStatsOSD.h RTEMS_MAJOR, RTEMS_MINOR, __RTEMS_REVISION are not defined. Is that why the version status queries do not work? defined in 6/powerpc-rtems6/beatnik/lib/include/rtems/score/cpuopts.h So rtems.h must be included before the RTEMS_XXX macros can be used?
But then I get this error:
support/StreamDevice/include -c ../os/RTEMS/osdFdUsage.c
../os/RTEMS/osdFdUsage.c: In function 'devIocStatsGetFDUsage':
../os/RTEMS/osdFdUsage.c:49:16: error: implicit declaration of function 'rtems_libio_count_open_iops' [-Werror=implicit-function-declaration]
49 | pval->used = rtems_libio_count_open_iops();
| ^~~~~~~
cc1: some warnings being treated as errors
make[2]: [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_BUILD:259: osdFdUsage.o] Error 1
make[2]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats/O.RTEMS-beatnik'
make[1]: [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_ARCHS:58: install.RTEMS-beatnik] Error 2
make[1]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats'
make: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_DIRS:85: devIocStats.install] Error 2
ok, found this: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/117#note_109499 Will update my RTEMS ...
The rtems.h
needs to be after #define __RTEMS_VIOLATE_KERNEL_VISIBILITY__
for the build to complete for the pre RTEMS 6 versions. I could have broken it for 6 making the change?
Works with the current RTEMS6 master(main). rtems_libio_count_open_iops is defined there.
Works with the current RTEMS6 master(main). rtems_libio_count_open_iops is defined there.
Thanks for confirming this. I do not know what the approval for workflows means.
@simon-ess will have to speak up if he cares about the clang-format failure (those checks seem rather obnoxious to me), but this looks good for the RTEMS builds, thanks!
I would prefer that the formatting check pass; otherwise it should be disabled, since we would then be just ignoring a failing test.
We could discuss exactly what the best formatting is, but I don't like the idea of letting it fail.
I would prefer that the formatting check pass; otherwise it should be disabled, since we would then be just ignoring a failing test.
I had no idea there was a coding standard. I am happy to take a look.
We could discuss exactly what the best formatting is, but I don't like the idea of letting it fail.
This makes sense. I can fix it.
I have pushed changes to fix the formatting. I did the changes manually from the diff in the CI log.
Is this OK?
I found a problem when I wanted to use iocStats for beaglebone black (arm). I had to change the following: diff --git a/devIocStats/os/RTEMS/osdClustInfo.c b/devIocStats/os/RTEMS/osdClustInfo.c index 1e07474..e1c74b7 100644 --- a/devIocStats/os/RTEMS/osdClustInfo.c +++ b/devIocStats/os/RTEMS/osdClustInfo.c @@ -110,6 +110,8 @@ int devIocStatsGetClusterInfo(int pool, clustInfo *pval) { return 0; }
+int devIocStatsGetClusterUsage(int pool, int *pval) { return -1; } +
/ This would otherwise need _KERNEL to be defined... /
This pull request adds support for RTEMS 6 for the legacy network and libbsd.
It needs a current build of RTEMS 6 and tools.