Skip to content

Commit 168c0ce

Browse files
Nissan Powclaude
andcommitted
prebuilt: address PR #92 council review (python_requires note + 4 clarity/robustness)
- python_requires>=3.10 (blocking): add a setup.py comment (both packages) that the floor mirrors the internal minimum (<3.10 dropped internally); no 3.10-only construct in the code — _parse_build_system_requires has tomllib/tomli/regex. - Pass B shared overlay: _overlay_install now warns when a build dep is re-staged at a DIFFERENT version (surfaces the documented shared-overlay conflict instead of a silent miscompilation). - deferred_sdists: comment that "url" is the guaranteed source (is_downloadable_url filter) and "filename" is only a best-effort hint (harmless if None). - _gather_embedded_wheels Case D/E: comment that p.url is a placeholder (is_real_url=False + cached wheel => p.url is never fetched). - Pass B overlay cleanup: comment that the on-failure rmtree + the finally rmtree double-remove is intentional and safe (ignore_errors=True). No functional change to validated paths (comments + one diagnostic warning). 77 prebuilt unit tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 87f7c2b commit 168c0ce

4 files changed

Lines changed: 34 additions & 1 deletion

File tree

metaflow-netflixext/setup.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
package_data={
5151
"metaflow_extensions.netflixext.plugins.conda.resources": ["*.png", "*.svg"]
5252
},
53+
# Mirrors the internal minimum (support for <3.10 was dropped internally).
5354
python_requires=">=3.10",
5455
install_requires=["metaflow>=2.16.0"],
5556
extras_require={

metaflow-prebuilt/metaflow_extensions/prebuilt/plugins/conda/prebuilt_build_install.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,9 @@ def _run_deferred_sdist_builds(env_dir, resolved_env): # type: ignore[no-untype
308308
build_env["PATH"] = (
309309
os.path.dirname(python_bin) + os.pathsep + build_env.get("PATH", "")
310310
)
311-
overlay_state = {"dir": None} # lazily created on first need
311+
# dir: overlay path (lazily created); staged: base-name -> the spec last staged,
312+
# used to warn when a build dep is re-staged at a different version (below).
313+
overlay_state = {"dir": None, "staged": {}}
312314

313315
def _ensure_overlay():
314316
if overlay_state["dir"] is None:
@@ -321,6 +323,23 @@ def _ensure_overlay():
321323
def _overlay_install(pkgs, what):
322324
if not pkgs:
323325
return
326+
# The overlay is SHARED across an image's deferred sdists. If two sdists
327+
# declare the same build dep at different versions, the last writer wins —
328+
# an earlier sdist may have built against the now-replaced version. We can't
329+
# isolate per-sdist (that fights --no-build-isolation), but we surface it
330+
# with a warning instead of a silent miscompilation.
331+
staged = overlay_state["staged"]
332+
for pkg in pkgs:
333+
base = re.split(r"[<>=!~ \[;]", pkg.strip(), 1)[0].strip().lower()
334+
prev = staged.get(base)
335+
if prev is not None and prev != pkg.strip():
336+
_echo(
337+
" WARNING: Pass B build dep %r re-staged as %r in the shared "
338+
"overlay (was %r); an earlier sdist may have built against the "
339+
"previous version." % (base, pkg.strip(), prev),
340+
timestamp=False,
341+
)
342+
staged[base] = pkg.strip()
324343
overlay = _ensure_overlay()
325344
_echo(" (staging %s) ..." % what, timestamp=False, nl=False)
326345
if os.path.isfile(uv_bin):
@@ -349,6 +368,8 @@ def _overlay_install(pkgs, what):
349368
] + list(pkgs)
350369
r = subprocess.run(args, capture_output=True, text=True)
351370
if r.returncode != 0:
371+
# Remove the partial overlay now; the outer finally also rmtrees it
372+
# (ignore_errors=True), so this double-remove is intentional and safe.
352373
shutil.rmtree(overlay, ignore_errors=True)
353374
raise CondaException(
354375
"Pass B: could not stage %s into the build overlay.\nstderr: %s"

metaflow-prebuilt/metaflow_extensions/prebuilt/plugins/conda/prebuilt_conda_environment.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,11 @@ def _get_or_build_image(
504504
# that wheel below, so they must NOT be deferred (the container would
505505
# otherwise rebuild from source, and only_binary=True would drop the
506506
# source archive in Pass A). Non-web and conda packages are excluded here.
507+
# "url" is the GUARANTEED build source — the is_downloadable_url() filter
508+
# below ensures it is a real, fetchable URL. "filename" is only a best-effort
509+
# hint: Pass B prefers the hash-verified local artifact matched by filename
510+
# and falls back to "url" when there's no match (so a None/empty filename is
511+
# harmless). Pass B raises only if BOTH are unusable.
507512
deferred_sdists: List[Any] = [
508513
{
509514
"name": p.package_name,
@@ -720,6 +725,9 @@ def _gather_embedded_wheels(
720725
wheel_fname = wheel_basename[: -len(cache_whl.format)]
721726
fetch_spec = PypiPackageSpecification(
722727
wheel_fname,
728+
# p.url is the SOURCE url and is only a placeholder: with
729+
# is_real_url=False + add_cached_version(".whl") below,
730+
# lazy_fetch reads the cached wheel and never fetches p.url.
723731
p.url,
724732
is_real_url=False, # built wheel: cache-only, never web
725733
url_format=".whl",

metaflow-prebuilt/setup.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@
3636
py_modules=[
3737
"metaflow_extensions",
3838
],
39+
# Mirrors the internal minimum (support for <3.10 was dropped internally). The
40+
# code has no 3.10-only construct — _parse_build_system_requires uses a
41+
# tomllib/tomli/regex fallback chain that works on any version.
3942
python_requires=">=3.10",
4043
install_requires=["metaflow>=2.16.0", "metaflow-netflixext>=1.3.12"],
4144
extras_require={

0 commit comments

Comments
 (0)