HBehrens / puncover

Analyses C/C++ build output for code size, static variables, and stack usage
MIT License
431 stars 94 forks source link

Interoperability between Windows and Linux paths with pathlib #64

Closed vChavezB closed 1 year ago

vChavezB commented 1 year ago

First of all thanks for the tool. It has helped agilize analyzing binaries instead of doing everything over the command line.

This pull requests addresses https://github.com/HBehrens/puncover/issues/37 and https://github.com/HBehrens/puncover/issues/62.

Motiviation

Its nice to use this tool in linux, but as at my workplace most of the computers have windows I wanted something more interoperable. I integrated pathlib to resolve paths independent of OS.

Unit tests

The unit tests should pass for both linux and windows. If someone has macOs, it would be nice to know if it also works.

Summary of differences

Final notes

I added myself in the license and as mantainer in the setup.py. If you have another suggestion for contact or authorship of these changes please let me know as I dont have experience with open-source projects :) . The main idea is that people can contact me in case they have problems with the changes I introduced.

Feel free to review the code and add suggestions so I can improve/modify the code. I will be honest I am more comfortable with Embedded C/C++ and python is just something I use for automating work flows at my work and hardly know all the syntax and latest best practices :)

Best Regards Victor

noahp commented 1 year ago

Hi @vChavezB , thanks for the patch! I've enabled tests for PRs here: https://github.com/HBehrens/puncover/pull/65

If you rebase, you can enable the tests on windows by doing this:

https://github.com/noahp/puncover/commit/2e275f496d3029cc4d2da7aeeb8a10c3d7b18ac3

I took your patch and applied it in a test branch here, but there seem to be some issues:

https://github.com/HBehrens/puncover/pull/67

Once those are passing, that should give us some confidence that things are working as expected!

vChavezB commented 1 year ago

Hi @noahp , I will check out why it failed with github actions. Briefly by checking the CI/CD I see that it uses python 3.6. My tests were done with python 3.11 , perhaps there is something that is different between these versions.

noahp commented 1 year ago

This change was merged in #70 , thanks a ton @vChavezB for the fix!