dolik-rce / pegof

PEG grammar optimizer and formatter
Other
9 stars 2 forks source link

Adding GitHub actions workflow #19

Open leleliu008 opened 1 month ago

leleliu008 commented 1 month ago

alpine:

# c
terminate called after throwing an instance of 'std::invalid_argument'
  what():  stoi
benchmark/run.sh: line 14:  2192 Aborted                 (core dumped) "$PEGOF" $OPTS --benchmark "benchmark/scripts/$BASE.sh" --output "/dev/null" -i "$GRAMMAR"
gmake[3]: *** [CMakeFiles/benchmark.dir/build.make:73: CMakeFiles/benchmark] Error 134
gmake[3]: Leaving directory '/__w/pegof/pegof/build'
gmake[2]: *** [CMakeFiles/Makefile2:197: CMakeFiles/benchmark.dir/all] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:204: CMakeFiles/benchmark.dir/rule] Error 2
gmake: *** [Makefile:224: benchmark] Error 2
leleliu008 commented 1 month ago

OpenBSD and NetBSD:

/home/runner/work/pegof/pegof/src/checker.h:20:88: error: '__sF' declared as array of references of type 'std::string &' (aka 'basic_string<char> &')
    bool call_packcc(const std::string& input, const std::string& output, std::string& stderr) const;
                                                                                       ^
/home/runner/work/pegof/pegof/amd[64](https://github.com/dolik-rce/pegof/actions/runs/9903002709/job/27357790089?pr=19#step:8:65)-unknown-netbsd-sysroot/usr/include/stdio.h:217:22: note: expanded from macro 'stderr'
#define stderr  (&__sF[2])
                      ^
/home/runner/work/pegof/pegof/src/checker.cc:53:15: error: out-of-line definition of 'call_packcc' does not match any declaration in 'Checker'
bool Checker::call_packcc(const std::string& input, const std::string& output, std::string& errors) const {
              ^~~~~~~~~~~
2 errors generated.
gmake[2]: *** [CMakeFiles/pegof.dir/build.make:307: CMakeFiles/pegof.dir/src/checker.cc.o] Error 1
gmake[2]: Leaving directory '/home/runner/work/pegof/pegof/build'
gmake[1]: *** [CMakeFiles/Makefile2:91: CMakeFiles/pegof.dir/all] Error 2
dolik-rce commented 1 month ago

OpenBSD and NetBSD:

/home/runner/work/pegof/pegof/src/checker.h:20:88: error: '__sF' declared as array of references of type 'std::string &' (aka 'basic_string<char> &')
    bool call_packcc(const std::string& input, const std::string& output, std::string& stderr) const;
                                                                                       ^
/home/runner/work/pegof/pegof/amd[64](https://github.com/dolik-rce/pegof/actions/runs/9903002709/job/27357790089?pr=19#step:8:65)-unknown-netbsd-sysroot/usr/include/stdio.h:217:22: note: expanded from macro 'stderr'
#define stderr  (&__sF[2])

Looks like the BSDs define macro called stderr... I'll rename the parameter to something else.

alpine:

# c
terminate called after throwing an instance of 'std::invalid_argument'
  what():  stoi
benchmark/run.sh: line 14:  2192 Aborted                 (core dumped) "$PEGOF" $OPTS --benchmark "benchmark/scripts/$BASE.sh" --output "/dev/null" -i "$GRAMMAR"

This happens because busybox implementation of time doesn't convert \n in format to newline... I'll try to think of some more universal way to measure memory consumption.

dolik-rce commented 1 month ago

Both problems should be now fixed in the devel branch. Please rebase, so we can see if any more errors pop up.

leleliu008 commented 1 month ago

filesystem header file is missing on NetBSD and OpenBSD. These platforms use old GCC version (prior to GCC-9.1) has no filesystem implementation yet. https://gcc.gnu.org/onlinedocs/gcc-13.1.0/libstdc++/manual/manual/status.html#iso.2014.filesystemts

I found a thirt-party implementation of filesystem https://github.com/gulrak/filesystem

leleliu008 commented 1 month ago

OpenBSD has experimental/filesystem

we should check filesystem and experimental/filesystem header file whether exist.

https://github.com/nlohmann/json/issues/3090

dolik-rce commented 1 month ago

I've pushed f4ffae424c3b20e154735af9204b55cb0b819168, which attempts to check if we should use <filesystem> or <experimental/filesystem>. Let's see if it helps...

dolik-rce commented 1 month ago

I notice this PR just keeps growing :slightly_smiling_face: I'd suggest to separate those platforms that already work (Alpine, FreeBSD and OpenBSD) to separate commit so I can cherry pick those and than we can tackle the remaining problems. Currently I see:

  1. failing tests on MacOS
  2. Android linking fails on snprintf
  3. NetBSD still fails on <filesystem>

    It might also be better to create separate issue for each, to keep the discussion cleaner. Would that be OK for you?

leleliu008 commented 1 month ago

2. Android linking fails on snprintf

because Android cmake toolchain file pass -D_FORTIFY_SOURCE=2 to clang, this will fore clang use static version of snprintf, but you do undefine static as per https://github.com/dolik-rce/pegof/blob/master/src/packcc_wrapper.c#L4 , which makes snprintf non-static.

diff --git a/src/packcc_wrapper.c b/src/packcc_wrapper.c
index 17f8c27..9631dfc 100644
--- a/src/packcc_wrapper.c
+++ b/src/packcc_wrapper.c
@@ -4,6 +4,9 @@
 #define static
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Woverlength-strings"
+#ifdef __ANDROID_API__
+#undef _FORTIFY_SOURCE
+#endif
 #include "packcc.c"
 #pragma GCC diagnostic push
 #undef static
dolik-rce commented 1 month ago

Thanks, I have cherry-picked the first three commits into devel.

I have also pushed 663b16ddbfd13401e9a7972b93332cb3166800dc, which should fix the android issues. Removing all the static keywords using preprocessor was an ugly hack and it was bound to cause trable like this. Current solution should by much less problematic in future.

leleliu008 commented 1 month ago

mkdtemp is missing on MSYS2 and MINGW

A reference implementation: https://github.com/microsoft/microsoft-r-open/blob/master/source/src/main/mkdtemp.c

dolik-rce commented 1 month ago

mkdtemp is missing on MSYS2 and MINGW

Should be fixed in ab8cafd862c0d7b55fc304e88d2fef730d55f2be. Unless it produces another incompatibility :slightly_smiling_face:

dolik-rce commented 1 month ago

@leleliu008: Please rebase on the devel branch. There is a commit c18bd64aeb1fb9a9cd3d62d1cd552359eb2a66d1, which fixes tests on NetBSD and I believe that it should resolve the issue with MacOS too.

leleliu008 commented 1 month ago

I am on a trip for the next few days. I will continue after I am back.

leleliu008 commented 1 month ago

@leleliu008: Please rebase on the devel branch. There is a commit c18bd64, which fixes tests on NetBSD and I believe that it should resolve the issue with MacOS too.

I'm back. I have rebased on devel branch.