fix: make preconfigure download paths consistent#2648
fix: make preconfigure download paths consistent#2648pcarruscag merged 5 commits intosu2code:developfrom
Conversation
Ensure preconfigure tooling saves downloaded archives next to the scripts that later extract/remove them (ninja-win.zip and module zips), making invocations from outside the repo root reliable. Also fix a NameError in the download error path and close zipfiles after extraction. Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
preconfigure.py
Outdated
| # If we are on windows, we don't need to compile ninja, we just download the executable | ||
| if os.name == "nt": | ||
| ninja_exe_url = "https://github.com/ninja-build/ninja/releases/download/v1.13.0/ninja-win.zip" | ||
| ninja_zip_path = sys.path[0] + os.path.sep + "ninja-win.zip" |
There was a problem hiding this comment.
isn't path.Join the pythonic way of doing it?
There was a problem hiding this comment.
os.path.join() / Path is the more idiomatic and safer way to build paths (cleaner, avoids manual separator handling, and stays cross-platform). I’ll switch this to os.path.join(sys.path[0], "ninja-win.zip") for consistency.
Does that work for your expectations here, or would you prefer I keep the current sys.path[0] + os.path.sep + ... style? If it’s not suitable, I can revert/adjust accordingly.
There was a problem hiding this comment.
Done , updated it to use os.path.join(sys.path[0], "ninja-win.zip")
Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
preconfigure.py
Outdated
| ) | ||
| shutil.copy(ninjapath + os.path.sep + "ninja", ".") | ||
| shutil.copy( | ||
| ninjapath + os.path.sep + "ninja", sys.path[0] + os.path.sep + "ninja" |
There was a problem hiding this comment.
Done — updated the shutil.copy() call to use os.path.join(...) for both src/dst, so the ninja binary is copied to the repo root (sys.path[0]) without relying on the current working directory.
meson_scripts/init.py
Outdated
| if not os.path.exists(filepath) and not os.path.exists(alt_filepath): | ||
| try: | ||
| urllib.request.urlretrieve(url, commit_sha + ".zip") | ||
| urllib.request.urlretrieve(url, filepath) |
There was a problem hiding this comment.
urlretrieve() now writes to filename again (commit_sha + ".zip"), which matches the previous behavior. We still compute filepath/alt_filepath for the later existence check/unzip. Verified locally with python -m compileall preconfigure.py meson_scripts/init.py.
Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
|
@pcarruscag Sir please check this also #2636 |
3 participants
Proposed Changes
This PR fixes path handling in the Meson/Ninja preconfigure tooling so downloads and extracted artifacts are placed relative to the script directory (not the current working directory). This makes
preconfigure.py/meson.pymore reliable when invoked from outside the repo root.Files Modified:
preconfigure.pyninja-win.zipinto the script directory; fix undefined exception variable in the download error path; ensure builtninjais copied into the script directory; close zip after extraction.meson_scripts/init.pyIssues Fixed
Preconfigure Tooling:
ninja-win.zipsaved to CWD but later opened/removed from script dirFileNotFoundErrorwhen runningpreconfigure.pyfrom outside repo root (Windows)sys.path[0]/ninja-win.zipand extract from the same path.NameErrorthat hides the real download failureexcept Exception as e:and print the actual exception.FileNotFoundErrorfor non-git source distributions or when using URL downloads from outside repo rootsys.path[0](expectedfilepath).with ZipFile(...) as zipf).ninjacopied to.instead of script dir (non-Windows)meson.pyexpectssys.path[0]/ninja, which can break if invoked from another working directorysys.path[0]/ninja.Related Work
This PR improves the build/preconfiguration workflow described in
README.md(Meson + Ninja viameson.py) by making downloads path-safe.PR Checklist