Skip to content

Ensure no missing pip dependencies in build_image.py #218

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Ensure no missing pip dependencies in build_image.py #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e75d1ed
Add `--check_python_reqs` option to build_image.py
ANogin Feb 8, 2023
42d6c87
Merge branch 'master' into feature/docker_python_deps_check
ANogin Jan 15, 2025
8a31e0e
Addressing PR feedback
ANogin Jan 15, 2025
43d3fca
Need to use a different workaround to `pip install -e`/mypy issues
ANogin Jan 15, 2025
2895cdc
Update build_image.py
ANogin Jan 16, 2025
b3ec507
Merge remote-tracking branch 'origin/master' into feature/docker_pyth…
ANogin Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions build_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ def create_dockerfile_base(config: OfrakImageConfig) -> str:
def create_dockerfile_finish(config: OfrakImageConfig) -> str:
full_base_image_name = "/".join((config.registry, config.base_image_name))
dockerfile_finish_parts = [
"# syntax = docker/dockerfile:1.3\n\n",
f"FROM {full_base_image_name}:{config.image_revision}\n\n",
f"ARG OFRAK_SRC_DIR=/\n",
]
Expand All @@ -273,13 +274,18 @@ def create_dockerfile_finish(config: OfrakImageConfig) -> str:
"\\n",
]
)
dockerfile_finish_parts.append(f'RUN printf "{develop_makefile}" >> Makefile\n')
dockerfile_finish_parts.append("RUN make $INSTALL_TARGET\n\n")
dockerfile_finish_parts += [
f'RUN printf "{develop_makefile}" >> Makefile\n'
'# We use --network="none" to ensure all dependencies were installed in base.Dockerfile\n',
'RUN --network="none" make $INSTALL_TARGET\n\n',
]
test_names = " ".join([f"test_{package_name}" for package_name in package_names])
finish_makefile = "\\n\\\n".join(
[
".PHONY: test " + test_names,
"test: " + test_names,
".PHONY: test inspect" + test_names,
"inspect:",
"\tpython3 -m pip check",
"test: inspect" + test_names,
]
+ [
f"test_{package_name}:\\n\\\n\t\\$(MAKE) -C {package_name} test"
Expand Down
4 changes: 3 additions & 1 deletion disassemblers/ofrak_angr/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ install:

.PHONY: develop
develop:
$(PIP) install -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install -e .[test]
Comment on lines +10 to +12
Copy link
Contributor

@whyitfor whyitfor Jan 16, 2025

Choose a reason for hiding this comment

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

I don't understand why this change is needed.

#561 updated these targets to enable editable_mode=compat -- is this not the same issue?

cc @alchzh

Copy link
Contributor

@alchzh alchzh Jan 16, 2025

Choose a reason for hiding this comment

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

yes this change is no longer needed (the command line flag is preferred over the environment variable)

Copy link
Collaborator Author

@ANogin ANogin Jan 16, 2025

Choose a reason for hiding this comment

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

No, with command line flag it insists on reaching out to pypi over network, but with the environment variable it does not.

Copy link
Contributor

@whyitfor whyitfor Jan 22, 2025

Choose a reason for hiding this comment

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

I've looked into this a bit and reflected.

If I understand correctly,SETUPTOOLS_ENABLE_FEATURES results in using a pre-PEP 517 backend. editable_mode=compat uses a PEP660 backend that provides some backwards compatibility. Switching back to SETUPTOOLS_ENABLE_FEATURES does not make sense to me: is moving the repo in the wrong direction, away from PEP660.

It looks like there are ways to get pip to not reach out to the network (a combination of --no-index and --find-links) which we might want to eventually look into.

For now though, I'd like to prioritize getting the pip check functionality this MR adds in, without the --network="none" changes.


.PHONY: inspect
inspect:
Expand Down
4 changes: 3 additions & 1 deletion disassemblers/ofrak_binary_ninja/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ install:

.PHONY: develop
develop:
$(PIP) install -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install -e .[test]

.PHONY: inspect
inspect:
Expand Down
4 changes: 3 additions & 1 deletion disassemblers/ofrak_capstone/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ install:

.PHONY: develop
develop:
$(PIP) install --pre -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install --pre -e .[test]

.PHONY: inspect
inspect:
Expand Down
4 changes: 3 additions & 1 deletion disassemblers/ofrak_ghidra/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ install:
$(PIP) install .

develop:
$(PIP) install -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install -e .[test]

test:
$(PYTHON) -m pytest --cov=ofrak_ghidra --cov-report=term-missing ofrak_ghidra_test
Expand Down
4 changes: 3 additions & 1 deletion ofrak_core/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ install: ofrak/gui/public

.PHONY: develop
develop: ofrak/gui/public
$(PIP) install -e .[docs,test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install -e .[docs,test]

.PHONY: inspect
inspect:
Expand Down
4 changes: 3 additions & 1 deletion ofrak_io/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ install:

.PHONY: develop
develop:
$(PIP) install -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install -e .[test]

.PHONY: inspect
inspect:
Expand Down
4 changes: 3 additions & 1 deletion ofrak_patch_maker/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ install:

.PHONY: develop
develop:
$(PIP) install -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install -e .[test]

.PHONY: inspect
inspect:
Expand Down
4 changes: 3 additions & 1 deletion ofrak_tutorial/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ run:

.PHONY: develop
develop:
${PIP} install -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" ${PIP} install -e .[test]
make generate_stripped_notebooks

.PHONY: install
Expand Down
4 changes: 3 additions & 1 deletion ofrak_type/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ install:

.PHONY: develop
develop:
$(PIP) install -e .[test] --config-settings editable_mode=compat
# Legacy-edinable is needed to allow mypy to find packages.
# See https://github.com/python/mypy/issues/13392
SETUPTOOLS_ENABLE_FEATURES="legacy-editable" $(PIP) install -e .[test]

.PHONY: inspect
inspect:
Expand Down
Loading