Skip to content

Don't use a context manager for _create_symlink_dir #178

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all 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
76 changes: 45 additions & 31 deletions pyodide_build/pypabuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,17 +284,23 @@ def make_command_wrapper_symlinks(symlink_dir: Path) -> dict[str, str]:
return env


# TODO: a context manager is no longer needed here
@contextmanager
def _create_symlink_dir(
env: dict[str, str],
build_dir: Path,
):
# Leave the symlinks in the build directory. This helps with reproducing.
symlink_dir = build_dir / "pywasmcross_symlinks"
shutil.rmtree(symlink_dir, ignore_errors=True)
symlink_dir.mkdir()
yield symlink_dir
def _create_symlink_dir(build_dir: Path | None) -> Path:
if build_dir is None:
# If no build directory is provided, create a temporary directory

# NOTE: This is not ideal and should not be merged as-is. We need
Copy link
Member

Choose a reason for hiding this comment

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

Is there anything that wrong with this? Temporary directories get removed on reboot anyways. Also wouldn't the alternative be... using a context manager?

# to find a better way to handle this.

import tempfile

symlink_dir = Path(tempfile.mkdtemp(prefix="pywasmcross_symlinks"))
else:
# Leave the symlinks in the build directory. This helps with reproducing.
symlink_dir = build_dir / "pywasmcross_symlinks"
shutil.rmtree(symlink_dir, ignore_errors=True)
symlink_dir.mkdir(exist_ok=True)

return symlink_dir


@contextmanager
Expand Down Expand Up @@ -327,28 +333,36 @@ def get_build_env(
args["exports"] = exports
env = env.copy()

with _create_symlink_dir(env, build_dir) as symlink_dir:
env.update(make_command_wrapper_symlinks(symlink_dir))
sysconfig_dir = Path(get_build_flag("TARGETINSTALLDIR")) / "sysconfigdata"
args["PYTHONPATH"] = sys.path + [str(symlink_dir), str(sysconfig_dir)]
args["orig__name__"] = __name__
args["pythoninclude"] = get_build_flag("PYTHONINCLUDE")
args["PATH"] = env["PATH"]
args["abi"] = get_build_flag("PYODIDE_ABI_VERSION")

pywasmcross_env = json.dumps(args)
# Store into environment variable and to disk. In most cases we will
# load from the environment variable but if some other tool filters
# environment variables we will load from disk instead.
env["PYWASMCROSS_ARGS"] = pywasmcross_env
(symlink_dir / "pywasmcross_env.json").write_text(pywasmcross_env)

env["_PYTHON_HOST_PLATFORM"] = platform()
env["_PYTHON_SYSCONFIGDATA_NAME"] = get_build_flag("SYSCONFIG_NAME")
env["PYTHONPATH"] = str(sysconfig_dir)
env["COMPILER_WRAPPER_DIR"] = str(symlink_dir)
symlink_dir = _create_symlink_dir(build_dir)

env.update(make_command_wrapper_symlinks(symlink_dir))

sysconfig_dir = Path(get_build_flag("TARGETINSTALLDIR")) / "sysconfigdata"
args["PYTHONPATH"] = sys.path + [str(symlink_dir), str(sysconfig_dir)]
args["orig__name__"] = __name__
args["pythoninclude"] = get_build_flag("PYTHONINCLUDE")
args["PATH"] = env["PATH"]
args["abi"] = get_build_flag("PYODIDE_ABI_VERSION")

pywasmcross_env = json.dumps(args)
# Store into environment variable and to disk. In most cases we will
# load from the environment variable but if some other tool filters
# environment variables we will load from disk instead.
env["PYWASMCROSS_ARGS"] = pywasmcross_env
(symlink_dir / "pywasmcross_env.json").write_text(pywasmcross_env)

env["_PYTHON_HOST_PLATFORM"] = platform()
env["_PYTHON_SYSCONFIGDATA_NAME"] = get_build_flag("SYSCONFIG_NAME")
env["PYTHONPATH"] = str(sysconfig_dir)
env["COMPILER_WRAPPER_DIR"] = str(symlink_dir)

try:
yield env
finally:
# Clean up a temporary directory, if we created one. Otherwise
# leave the symlinks in the build directory for reproducibility.
if build_dir is None:
shutil.rmtree(symlink_dir, ignore_errors=True)


def build(
Expand Down