-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[red-knot] Typeshed patching: use build.rs instead of workflow #15370
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
//! in `crates/red_knot_vendored/vendor/typeshed` change. | ||
|
||
use std::fs::File; | ||
use std::io::Write; | ||
use std::path::Path; | ||
|
||
use path_slash::PathExt; | ||
|
@@ -14,6 +15,7 @@ use zip::write::{FileOptions, ZipWriter}; | |
use zip::CompressionMethod; | ||
|
||
const TYPESHED_SOURCE_DIR: &str = "vendor/typeshed"; | ||
const KNOT_EXTENSIONS_STUBS: &str = "knot_extensions/knot_extensions.pyi"; | ||
const TYPESHED_ZIP_LOCATION: &str = "/zipped_typeshed.zip"; | ||
|
||
/// Recursively zip the contents of an entire directory. | ||
|
@@ -55,21 +57,32 @@ fn zip_dir(directory_path: &str, writer: File) -> ZipResult<File> { | |
// Some unzip tools unzip files with directory paths correctly, some do not! | ||
if absolute_path.is_file() { | ||
println!("adding file {absolute_path:?} as {normalized_relative_path:?} ..."); | ||
zip.start_file(normalized_relative_path, options)?; | ||
zip.start_file(&*normalized_relative_path, options)?; | ||
let mut f = File::open(absolute_path)?; | ||
std::io::copy(&mut f, &mut zip).unwrap(); | ||
|
||
// Patch the VERSIONS file to make `knot_extensions` available | ||
if normalized_relative_path == "stdlib/VERSIONS" { | ||
writeln!(&mut zip, "knot_extensions: 3.0-")?; | ||
} | ||
} else if !normalized_relative_path.is_empty() { | ||
// Only if not root! Avoids path spec / warning | ||
// and mapname conversion failed error on unzip | ||
println!("adding dir {absolute_path:?} as {normalized_relative_path:?} ..."); | ||
zip.add_directory(normalized_relative_path, options)?; | ||
} | ||
} | ||
|
||
// Patch typeshed and add the stubs for the `knot_extensions` module | ||
println!("adding file {KNOT_EXTENSIONS_STUBS} as stdlib/knot_extensions.pyi ..."); | ||
zip.start_file("stdlib/knot_extensions.pyi", options)?; | ||
let mut f = File::open(KNOT_EXTENSIONS_STUBS)?; | ||
std::io::copy(&mut f, &mut zip).unwrap(); | ||
|
||
zip.finish() | ||
} | ||
|
||
fn main() { | ||
println!("cargo::rerun-if-changed={TYPESHED_SOURCE_DIR}"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing this means that we will rebuild And I don't think there's a huge drawback since there are no files in |
||
assert!( | ||
Path::new(TYPESHED_SOURCE_DIR).is_dir(), | ||
"Where is typeshed?" | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I originally wrote this function, I intended to write it so that it could be used to zip up any directory into a zip archive -- that's why it's called
zip_dir()
, and that's why it accepts the typeshed directory path as a parameter rather than hardcoding it in the function. I think since I originally wrote it, it's already grown a few features that make it quite specific to its use-case here, such as the conditional logic a few lines above for deciding what the compression method should be. So I don't really mind much hardcoding things like"stdlib/VERSIONS"
directly into the function body. But at this stage, it probably doesn't make much sense for the typeshed directory to be passed into the function as a parameter, and we should probably change the name of the function 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that, I figured it would be okay to modify in this way since it wasn't being used anywhere else and not in a place where it could be imported from anywhere else. See #15372