Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interoperability between Windows and Linux paths with pathlib #64

Closed
wants to merge 2 commits into from

Conversation

vChavezB
Copy link
Contributor

@vChavezB vChavezB commented Dec 10, 2022

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 #37 and #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

  • Use of pathlib to avoid path compatibility between OS´s
  • Changed tests for Collector class with pathlib.
  • Fixed stack usage base file path, which is similar to what I did in this separate pull request Fix stack usage parsing of base file name #63
  • Added copyright and mantainer info for further contact of the changes of this commit.

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

unknown and others added 2 commits December 11, 2022 00:30
- Changed tests for Collector class with pathlib.
- Added copyright and mantainer info for further contact of the changes of this commit.
@noahp
Copy link
Collaborator

noahp commented Dec 20, 2022

Hi @vChavezB , thanks for the patch! I've enabled tests for PRs here:
#65

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

noahp@2e275f4

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

#67

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

@vChavezB
Copy link
Contributor Author

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
Copy link
Collaborator

noahp commented Mar 2, 2023

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

@noahp noahp closed this Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants