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

NewPyBuildSupport: Add build matrix entries for 3.11 and 3.12 #77

Merged
merged 4 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
16 changes: 16 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,19 @@ jobs:
run: docker build . --target=test --build-arg PYTHON_VERSION=${PYTHON_VERSION}
env:
PYTHON_VERSION: "3.10"
build-311:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Run unit tests
run: docker build . --target=test --build-arg PYTHON_VERSION=${PYTHON_VERSION}
env:
PYTHON_VERSION: "3.11"
build-312:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Run unit tests
run: docker build . --target=test --build-arg PYTHON_VERSION=${PYTHON_VERSION}
env:
PYTHON_VERSION: "3.12"
78 changes: 58 additions & 20 deletions import_tracker/import_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import dis
import importlib
import os
import re
import sys

# Local
Expand Down Expand Up @@ -250,12 +251,13 @@ def _get_dylib_dir():
sample_dylib = fname
break

if sample_dylib is not None:
return os.path.realpath(os.path.dirname(sample_dylib))

# If all else fails, we'll just return a sentinel string. This will fail to
# match in the below check for builtin modules
return "BADPATH" # pragma: no cover
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Trying to get rid of # pragma: no cover here with a little inlining

return (
os.path.realpath(os.path.dirname(sample_dylib))
if sample_dylib is not None
else "BADPATH"
)


# The path where global modules are found
Expand Down Expand Up @@ -330,9 +332,39 @@ def _get_op_number(dis_line: str) -> Optional[int]:
return int(line_parts[opcode_idx - 1])


def _get_try_end_number(dis_line: str) -> int:
"""For a SETUP_FINALLY/SETUP_EXPECT line, extract the target end line"""
return int(_get_value_col(dis_line).split()[-1])
def _get_try_end_number(
dis_line: str,
op_num: Optional[int],
exception_table: Dict[int, int],
) -> Optional[int]:
"""If the line contains a known indicator for a try block, get the
corresponding end number

NOTE: This contains compatibility code for changes between 3.10 and 3.11
"""
return exception_table.get(op_num or -1) or (
int(_get_value_col(dis_line).split()[-1])
if any(op in dis_line for op in ["SETUP_FINALLY", "SETUP_EXCEPT"])
else None
)


def _get_exception_table(dis_lines: List[str]) -> Dict[int, int]:
"""For 3.11+ exception handling, parse the Exception Table"""
table_start = [i for i, line in enumerate(dis_lines) if line == "ExceptionTable:"]
assert len(table_start) <= 1, "Found multiple exception tables!"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have not encountered a case where this is larger than 1 and can't imagine a circumstance under which it would be, but I haven't exhaustively read the spec either, so if this ever trips, we may need to investigate this further.

return (
{
int(m.group(1)): int(m.group(2))
for m in [
re.match(r" ([0-9]+) to ([0-9]+) -> [0-9]+ \[([0-9]+)\].*", line)
for line in dis_lines[table_start[0] + 1 :]
]
if m and int(m.group(3)) == 0 and m.group(1) != m.group(2)
}
if table_start
else {}
)


def _figure_out_import(
Expand Down Expand Up @@ -377,7 +409,7 @@ def _figure_out_import(
log.debug3("Module file: %s", getattr(mod, "__file__", None))
if not import_name:
import_name = root_mod_name
else:
elif root_mod_name:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change showed up as I was trying to understand why some modules were not identified as optional. I believe this was a bug since the resulting name could be ".foobar" which would not show up in sys.modules when "foobar" would.

import_name = f"{root_mod_name}.{import_name}"

# Try with the import_from attached. This might be a module name or a
Expand Down Expand Up @@ -419,15 +451,23 @@ def _get_imports(mod: ModuleType) -> Tuple[Set[ModuleType], Set[ModuleType]]:
open_import = False
open_tries = set()
log.debug4("Byte Code:")
for line in bcode.dis().split("\n"):
dis_lines = bcode.dis().split("\n")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't love the need to iterate the lines twice, but this should be a pretty small performance penalty


# Look for and parse an Exception Table (3.11+)
exception_table = _get_exception_table(dis_lines)
log.debug4("Exception Table: %s", exception_table)

for line in dis_lines:
log.debug4(line)
line_val = _get_value_col(line)

# Check whether this line ends a try
# If this is the beginning of a try block, add the end to the known open
# try set
op_num = _get_op_number(line)
if op_num in open_tries:
open_tries.remove(op_num)
log.debug3("Closed try %d. Remaining open tries: %s", op_num, open_tries)
try_end = _get_try_end_number(line, op_num, exception_table)
if try_end:
open_tries.add(try_end)
log.debug3("Open tries: %s", open_tries)

# Parse the individual ops
if "LOAD_CONST" in line:
Expand All @@ -440,13 +480,6 @@ def _get_imports(mod: ModuleType) -> Tuple[Set[ModuleType], Set[ModuleType]]:
open_import = True
current_import_from = line_val
else:
# If this is a SETUP_FINALLY (try:), increment the number of try
# blocks open
if "SETUP_FINALLY" in line or "SETUP_EXCEPT" in line:
# Get the end target for this try
open_tries.add(_get_try_end_number(line))
log.debug3("Open tries: %s", open_tries)

# This closes an import, so figure out what the module is that is
# being imported!
if open_import:
Expand All @@ -473,6 +506,11 @@ def _get_imports(mod: ModuleType) -> Tuple[Set[ModuleType], Set[ModuleType]]:
open_import = False
current_import_from = None

# Close the open try if this ends one
if op_num in open_tries:
open_tries.remove(op_num)
log.debug3("Closed try %d. Remaining open tries: %s", op_num, open_tries)

# To the best of my knowledge, all bytecode will end with something other
# than an import, even if an import is the last line in the file (e.g.
# STORE_NAME). If this somehow proves to be untrue, please file a bug!
Expand Down
2 changes: 1 addition & 1 deletion test/test_import_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ def test_optional_deps():
}


def test_updatream_optional_deps():
def test_upstream_optional_deps():
"""Make sure that a module which holds a third-party dep as optional where
that third-party dep includes _other_ third-party deps as non-optional
should have the transitive deps held as optional due to the optional dep in
Expand Down
Loading