jamescourtney / FlatSharp

Fast, idiomatic C# implementation of Flatbuffers
Apache License 2.0
497 stars 50 forks source link

Fix ExperimentalBenchmark and FlatSharpEndToEndTests on Linux and macOS #258

Closed 0xced closed 2 years ago

0xced commented 2 years ago

Using properties and items makes sure that paths with backslashes are properly interpreted as paths on Linux and macOS. Using \ directly inside the Exec task would produce the following error:

FlatSharpEndToEndTests.csproj(44, 5): [MSB3073] The command "dotnet ....\FlatSharp.Compiler\bin\Debug\net5.0\FlatSharp.Compiler.dll --nullable-warnings true -i Grpc\Grpc.fbs -o Grpc\" exited with code 1.

Also, the flatc binaries must have executable permissions set on Linux and macOS else we get this exception:

System.ComponentModel.Win32Exception (13): Permission denied

Finally, the FBS target must be run before the CoreCompile target instead of the BeforeBuild target to ensure that FlatSharp.Compiler.dll has been built. You see this error when doing a fresh checkout of the project when the FlatSharp.Compiler project has not been built yet.

jamescourtney commented 2 years ago

Thanks for this -- can you explain the change to flatc? Is it just chmod +x?

codecov[bot] commented 2 years ago

Codecov Report

Merging #258 (9a3d42f) into master (ca0c9f6) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #258   +/-   ##
=======================================
  Coverage   95.25%   95.25%           
=======================================
  Files         109      109           
  Lines        7168     7168           
  Branches      673      673           
=======================================
  Hits         6828     6828           
  Misses        237      237           
  Partials      103      103           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ca0c9f6...9a3d42f. Read the comment docs.

0xced commented 2 years ago

Yes, exactly, just chmod +x.

jamescourtney commented 2 years ago

Yes, exactly, just chmod +x.

I'm a little confused then. I don't have a Mac, but when I added flatc, I did explicitly verify that the compiler did correctly work on Linux in a VM, and I didn't need chmod then. Not saying you're wrong or anything -- just a bit weird.

This looks fine to me, though I do need to do a hex diff of the flatc binaries.

jamescourtney commented 2 years ago

Oh cool, I've learned something! I spent about 20 minutes doing sha256 checksums of the flatc binaries and wondering why it wasn't changing. I didn't realize that the change you made wasn't to the binary itself, but to the permissions that git applies to the files on checkout, hence the 100644 → 100755 change to apply +x.

Thanks for your contribution! Be sure and let me know if you find a good use for FlatSharp. I'm always interested to learn how people are using it :)