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

Circular dependency cases a make develop weirdness. #419

Open
ANogin opened this issue Feb 12, 2024 · 2 comments
Open

Circular dependency cases a make develop weirdness. #419

ANogin opened this issue Feb 12, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@ANogin
Copy link
Contributor

ANogin commented Feb 12, 2024

What is the problem? (Here is where you provide a complete Traceback.)

#226 introduced an unfortunate issue (which I discovered using PR #218 - IMHO we really out to land that one, and use in CI) - when the docker build runs make develop, it would first go to ofrak_core, where the new "test": "ofrak_angr~=1.0", "ofrak_capstone~=1.0"] dependency would cause it to download ofrak_angr from PyPI, then it would go into ofrak_angr and then the make develop there would overwrite the downloaded one with the local one (and similar with ofrak_capstone). With #417, this means that not only the ofrak_angr itself is downloaded from PyPI, but also the angr itself and all its dependencies installed in base.Dockerfile are updated to the versions required by the ofrak_angr from PyPI. Then make develop would put them all back.

Please provide some information about your environment.
N/A

If you've discovered it, what is the root cause of the problem?
See above.

How often does the issue happen?
Always.

What are the steps to reproduce the issue?
See above.

How would you implement this fix?
Ideally, we ought to get rid of the "test": "ofrak_angr~=1.0", "ofrak_capstone~=1.0"] dependency. But no idea what that would take.

Are there any (reasonable) alternative approaches?

Options:

  • Leave as is, and give up on being able to install all dependencies in base.Dockerfile and not rely on PyPI in make develop afterwards. Seems pretty suboptimal in the long run…
  • Implement a way to have the top-level make develop first pip install -e . without [test], in ofrak_core, then make develop in ofrak_angr, then go back and do a full make develop in ofrak_core. I see three ways, none ideal:
    • Change the develop target in ofrak_core/Makefile to do this when ../ofrak_angr exists. The downside is that this has a very unexpected side-effect for people calling this make develop for other reasons
    • Have the code in top-level build_image.py for making the "$INSTALL_TARGET: rule in top-level Makefile now do this special thing for ofrak_core (that is, when calling make develop for each component in order, call make develop_no_test in ofrak_core instead, then go back and call make -C ofrak_core develop at the end). OK, but special-case'ing ofrak_core is not very clean.
    • Have the top-level build_image.py create a top-level Makefile do two passes through each components - first do make -C {package_name) develop1 for each package, then make -C {package_name) develop2, and change all package makefiles to have develop1: develop and develop2: while ofrak_core would have develop1 do pip install -e . and develop2:develop. Does not hardcode a special case for ofrak_core, but requires changing all the package Makefiles...

Are you interested in implementing it yourself?
Not for the ideal approach - do not know enough. I could implement one of the alternative approaches, but do not know which would be preferred.

@ANogin
Copy link
Contributor Author

ANogin commented Feb 12, 2024

Hm, it seems that simply moving these requirements from ofrak_core/setup.py to ofrak_core/requirements-test.txt would address most of the negative effects of this - this way, the ofrak_angr and ofrak_capstone would still be downloaded from PyPI, but it will happen once in base.Dockerfile, and no further PyPI interaction would happen at the make develop stage. (This would be undoing #294 in that respect - not sure why @EdwardLarson wanted/needed to move these dependencies). That said, if the local ofrak_angr/ofrak_capstone have requirements that are incompatible with the PyPI ones, then they will likely still "fight" during make develop.

@EdwardLarson @whyitfor any thoughts?

[Edited to add: I implemented this approach in #420 and it works there, but when merged with #417, then during make develop, there is indeed a "fight" about the version of angr to install.]

@rbs-jacob
Copy link
Member

Chiming in to add that I have raised this internally to @whyitfor, and I second that it needs to be addressed. We did not come to any conclusions in our disucssion about a clear path forward.

@whyitfor whyitfor added the bug Something isn't working label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants