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

Fixing build system to use setuptools and wheel install #376

Closed
wants to merge 5 commits into from

Conversation

E3V3A
Copy link
Contributor

@E3V3A E3V3A commented Nov 29, 2018

  • fixes issue Fixing build system #336
  • Added build artifact src/binwalk.egg-info to .gitignore
  • Updated INSTALL.md to use pip install .
  • Added setup.cfg for pip
  • Changed setup.py to comply to pip setuptools install
  • Copied binwalk.py to src/binwalk/main.py

* fixes issue ReFirmLabs#336
* Added build artifact src/binwalk.egg-info to .gitignore
* Updated INSTALL.md to use pip install .
* Added setup.cfg for pip
* Changed setup.py to comply to pip setuptools install
* Copied binwalk.py to src/binwalk/__main__.py
setup.cfg Outdated
setuptools

#test_suite = tests.tests.Tests
dependency_links = ['git+https://github.com/devttys0/yaffshiv.git']
Copy link
Contributor

@KOLANICH KOLANICH Nov 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove. When I have written the patch, dependency_links were supported, now they are dropped in favour of PEP 508.

[options.extras_require]
plots = matplotlib
x86 = capstone
YAFFS = yaffshiv
Copy link
Contributor

@KOLANICH KOLANICH Nov 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be yaffshiv @ git+https://github.com/devttys0/yaffshiv.git but currently it doesn't work because of the bug in pip which is not fixed even in master.

@KOLANICH
Copy link
Contributor

Copied binwalk.py to src/binwalk/__main__.py

Moved.

@E3V3A
Copy link
Contributor Author

E3V3A commented Nov 29, 2018

IDK what's the problem with Travis... :(
Seem like all tests are failing, so I suppose we can ignore as usual?

setup.cfg Outdated

[options.extras_require]
plots = matplotlib
x86 = capstone
YAFFS = yaffshiv
#SQUASH = sasquatch
#YAFFS = yaffshiv @ git+https://github.com/devttys0/yaffshiv.git
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks things. You need to comment out the part starting from @, not to comment out the line entirely.

setup.cfg Outdated
YAFFS = yaffshiv
#SQUASH = sasquatch
#YAFFS = yaffshiv @ git+https://github.com/devttys0/yaffshiv.git
#SQUASH = sasquatch @ git+https://github.com/devttys0/sasquatch.git'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have forgotten the tick

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I just remove them altogether then?

Copy link
Contributor

@KOLANICH KOLANICH Nov 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, no. YAFFS should be kept - 3rd party packages may depend on it, if you removed this optional dependency, they would be broken.

I prefer to keep these commented out lines as a reminder to fix them in future when the bug in pip will be fixed.

@KOLANICH
Copy link
Contributor

IDK what's the problem with Travis... :(
Seem like all tests are failing, so I suppose we can ignore as usual?

Travis has lots of problems, they use obsolete ubuntu trusty, all my packages have failed to be built there, we should use other CI like GitLab or Appveyor.

@KOLANICH
Copy link
Contributor

KOLANICH commented Nov 29, 2018

Don't forget to squash all the commits into a single one, rewriting the history.

@E3V3A
Copy link
Contributor Author

E3V3A commented Nov 29, 2018

Don't forget to squash all the commits into a single one, rewriting history.

isn't that done automagically? (I can't find the button for that.)

@KOLANICH
Copy link
Contributor

KOLANICH commented Nov 29, 2018

@E3V3A, It's done on your computer, not on GitHub.

@E3V3A
Copy link
Contributor Author

E3V3A commented Nov 29, 2018

@KOLANICH
Yeah, sorry IDK the squash since I have an unusual workfflow.
Form PC to my GH then here.
My GH binwalk is set to squash, so how would I do it straight from the computer?
(To me it look like this PR is squashed when it arrives here...no?)

@E3V3A
Copy link
Contributor Author

E3V3A commented Nov 29, 2018

Anyway, I can't do this all day/night. So since it seem that other people know what exactly to do, so please do it. I was just hoping to help out and make things move. So please merge and fix or whatever is necessary.

Surely the setup.py need a lot more maintenance. It's almost unreadable as it is right now, and I think the distutils import may need to be replaced, but I could be wrong. I'm quite new to packing...

setup.py Outdated
@@ -7,11 +11,18 @@
from distutils.core import setup, Command
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ouch. setup, should be removedk we use setuptools setup

@E3V3A
Copy link
Contributor Author

E3V3A commented Nov 29, 2018

This PR is so embarrassingly ugly I'm closing it to send a new one!

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