Fix circular dependencies by making `ofrak_angr` and `ofrak_capstone` optional for `ofrak_core` tests

[ANogin]

• [X] I have reviewed the OFRAK contributor guide and attest that this pull request is in accordance with it.

One sentence summary of this PR (This should go in the CHANGELOG!) Optmizes the build of the --target=development docker containers for the ofrak_core "forward" test dependencies on ofrak_angr and ofrak_capstone

Link to Related Issue(s)

419

Please describe the changes in your request. Split the make develop for ofrak_core into two stages, to have the install process agree with the dependency order.

Anyone you think should look at this, specifically? @whyitfor

[ANogin]

While this works on master, when merged with #417, then during make develop, there is still a "fight" about the version of angr to install, so I am no longer sure this is an OK way to go for #419, marked as draft for now...

[ANogin]

@whyitfor @rbs-jacob does this approach make sense?

[ANogin]

Specifically, this results in something like

develop:
        $(MAKE) -C ofrak_type STAGE=1 develop
        $(MAKE) -C ofrak_io STAGE=1 develop
        $(MAKE) -C ofrak_patch_maker STAGE=1 develop
        $(MAKE) -C ofrak_core STAGE=1 develop
        $(MAKE) -C ofrak_angr STAGE=1 develop
        $(MAKE) -C ofrak_capstone STAGE=1 develop
        $(MAKE) -C frontend STAGE=1 develop
        $(MAKE) -C ofrak_core STAGE=2 develop

where $(MAKE) -C ofrak_core STAGE=1 develop would $(PIP) install -e . (without circular dependencies), and then $(MAKE) -C ofrak_core STAGE=2 develop would $(PIP) install -e .[docs,test] (with circular dependencies, but they are already installed, so no issue).

[ANogin]

Could the same end result be achieved by modifying the ofrak develop target to ensure proper install order?

Something like:

develop:
    $(PIP) install -e .
    $(PIP) install -e .[docs,test]

No, if would have to be something like

develop:
     $(PIP) install -e .
     $(MAKE) -C disassemblers/ofrak_capstone develop
     $(MAKE) -C disassemblers/angr develop
     $(PIP) install -e .[docs,test]

Would that be OK?

Alternatively, should the develop target by default install all ofrak-related dependencies from the source repository (and not from pypi)? I believe @rbs-jacob has suggested such a solution as being advantageous for development as well

What would be the approach for that? One that would probably work is:

• In the top-level Makefile, have make develop simply do pip install -e path1 path2 path3...." for all OFRAK paths before the existing$(MAKE) -C path1 develop; ${MAKE) -C path2 develop; ...`, except that it would require that all OFRAK modules are PIP modules (I think frontend is currently the main exception).

[ANogin]

@whyitfor , I implemented an alternative approach where only the make develop target in ofrak_core/Makefile changes - does this work?

P.S. The reson for the other two changes is that:

• root make image rule is changed because ofrak-core-dev.yml no longer makes sense, as a minimal dev install needs to include capstone and angr (that is, ofrak-angr.yml)
• importlib-resources moved from setup to requirements so that it is installed in the base.Dockerfile stage (there because my test for this fix working right is that the full make develop suceeeds without needing network access)

[whyitfor]

@ANogin, can you pull master into this for the CI tests?

[ANogin]

@whyitfor , note my earlier caveat:

root make image rule is changed because ofrak-core-dev.yml no longer makes sense, as a minimal dev install needs to include capstone and angr (that is, ofrak-angr.yml)

So reverting this change is not the right thing to do.

[ANogin]

@whyitfor in order for things to still [kind of] work with ofrak-core-dev.yml, I tried an alternative approach, which I think is cleaner - the ofrak_angr and ofrak_capstone are not included as dependencies, and the tests that do rely on them are just skipped if they are not installed. The result is that make -C /ofrak_core test succeeds in e.g. ofrak-angr.yml but in ofrak-core-dev.yml while all tests succeed, the coverage check then fails with FAIL Required function coverage of 100.00% not reached. Total coverage: 99.91% (because of the skipped tests).