Skip to content

fix: functions push producing empty python_bundle.sources on Windows #259

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.

Sign up for GitHub

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
Cedric / ViaDézo1er (viadezo1er) wants to merge 1 commit into
base: main
Choose a base branch
from
+87 −39
Open

fix: functions push producing empty python_bundle.sources on Windows #259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 15 additions & 8 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ oauth2 = { version = "4.4", default-features = false }
getrandom = "0.3"
chrono = { version = "0.4.40", features = ["clock"] }
dirs = "5"
dunce = "1"
pathdiff = "0.2.3"
glob = "0.3"
flate2 = "1.1.2"
Expand Down
3 changes: 2 additions & 1 deletion scripts/functions-runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
purge_local_modules,
python_version,
resolve_module_info,
strip_extended_length_prefix,
)


Expand Down Expand Up @@ -286,7 +287,7 @@ async def collect_function_event_entries(prompts_registry: Any) -> list[dict[str


async def process_file(file_path: str) -> dict[str, Any]:
abs_path = os.path.abspath(file_path)
abs_path = os.path.abspath(strip_extended_length_prefix(file_path))
cwd = os.getcwd()
if cwd not in sys.path:
sys.path.insert(0, cwd)
Expand Down
20 changes: 17 additions & 3 deletions scripts/python_runner_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,28 @@
from types import ModuleType


def strip_extended_length_prefix(path: str) -> str:
r"""Strip a Windows ``\\?\`` extended-length (verbatim) prefix.

Verbatim drives (``\\?\C:``) read as different from ``C:`` under
``os.path.commonpath``, so cwd comparisons drop every source.
"""
if path.startswith("\\\\?\\UNC\\"):
return "\\\\" + path[len("\\\\?\\UNC\\") :]
if path.startswith("\\\\?\\"):
return path[len("\\\\?\\") :]
return path


def normalize_file_list(files: list[str]) -> list[str]:
unique: set[str] = set()
for file_path in files:
unique.add(os.path.abspath(file_path))
unique.add(os.path.abspath(strip_extended_length_prefix(file_path)))
return sorted(unique)


def resolve_module_info(in_file: str) -> tuple[str, list[str]]:
in_file = os.path.abspath(in_file)
in_file = os.path.abspath(strip_extended_length_prefix(in_file))
module_dir = os.path.dirname(in_file)
module_name = os.path.splitext(os.path.basename(in_file))[0]

Expand Down Expand Up @@ -71,7 +84,8 @@ def purge_local_modules(cwd: str, preserve_modules: set[str] | None = None) -> N

def collect_python_sources(cwd: str, input_file: str) -> list[str]:
sources: set[str] = set()
input_abs = os.path.abspath(input_file)
cwd = strip_extended_length_prefix(cwd)
input_abs = os.path.abspath(strip_extended_length_prefix(input_file))
sources.add(input_abs)

for module in list(sys.modules.values()):
Expand Down
79 changes: 52 additions & 27 deletions src/functions/push.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,7 @@ fn collect_classified_files(inputs: &[PathBuf]) -> Result<ClassifiedFiles> {
let mut python = BTreeSet::new();
let mut allowed_roots = BTreeSet::new();
if let Ok(cwd) = std::env::current_dir() {
if let Ok(canonical_cwd) = cwd.canonicalize() {
if let Ok(canonical_cwd) = dunce::canonicalize(&cwd) {
allowed_roots.insert(canonical_cwd);
}
}
Expand All @@ -1183,8 +1183,7 @@ fn collect_classified_files(inputs: &[PathBuf]) -> Result<ClassifiedFiles> {

if path.is_file() {
explicit_file_inputs += 1;
let canonical = path
.canonicalize()
let canonical = dunce::canonicalize(&path)
.with_context(|| format!("failed to canonicalize file {}", path.display()))?;
let parent = canonical
.parent()
Expand All @@ -1207,8 +1206,7 @@ fn collect_classified_files(inputs: &[PathBuf]) -> Result<ClassifiedFiles> {
continue;
}

let canonical_dir = path
.canonicalize()
let canonical_dir = dunce::canonicalize(&path)
.with_context(|| format!("failed to canonicalize directory {}", path.display()))?;
allowed_roots.insert(canonical_dir.clone());
collect_from_dir(&canonical_dir, &mut js_like, &mut python)?;
Expand Down Expand Up @@ -1259,8 +1257,7 @@ fn collect_from_dir_inner(
if file_type.is_dir() && !file_type.is_symlink() {
collect_from_dir_inner(&path, js_like, python, depth + 1)?;
} else if file_type.is_file() {
let canonical = path
.canonicalize()
let canonical = dunce::canonicalize(&path)
.with_context(|| format!("failed to canonicalize file {}", path.display()))?;
match classify_source_file(&canonical) {
Some(SourceLanguage::JsLike) => {
Expand Down Expand Up @@ -1337,9 +1334,8 @@ fn validate_manifest_paths(
let mut seen = BTreeSet::new();

for file in &manifest.files {
let path = PathBuf::from(&file.source_file)
.canonicalize()
.map_err(|err| FileFailure {
let path =
dunce::canonicalize(PathBuf::from(&file.source_file)).map_err(|err| FileFailure {
reason: HardFailureReason::ManifestPathMissing,
message: format!("manifest source file missing: {} ({err})", file.source_file),
})?;
Expand Down Expand Up @@ -1424,7 +1420,7 @@ fn validate_python_bundle(

let mut sources = BTreeSet::new();
for raw_source in &python_bundle.sources {
let canonical = PathBuf::from(raw_source).canonicalize().with_context(|| {
let canonical = dunce::canonicalize(PathBuf::from(raw_source)).with_context(|| {
format!(
"manifest file '{}' referenced missing python source {}",
manifest_file.source_file, raw_source
Expand Down Expand Up @@ -2030,8 +2026,7 @@ fn collect_regular_files_recursive_impl(
}

fn validate_requirements_path(path: &Path, allowed_roots: &[PathBuf]) -> Result<PathBuf> {
let canonical = path
.canonicalize()
let canonical = dunce::canonicalize(path)
.with_context(|| format!("requirements file not found: {}", path.display()))?;
if !canonical.is_file() {
bail!("requirements path is not a file: {}", canonical.display());
Expand Down Expand Up @@ -2172,8 +2167,7 @@ fn resolve_requirement_path(reference: &str, parent: &Path) -> Result<PathBuf> {
} else {
parent.join(candidate)
};
absolute
.canonicalize()
dunce::canonicalize(&absolute)
.with_context(|| format!("failed to resolve requirements reference {}", normalized))
}

Expand Down Expand Up @@ -3164,8 +3158,8 @@ mod tests {
let dir = tempfile::tempdir().expect("tempdir");
let source = dir.path().join("tool.js");
std::fs::write(&source, "export const x = 1;\n").expect("write source file");
let source = source.canonicalize().expect("canonicalize source");
let root = dir.path().canonicalize().expect("canonicalize root");
let source = dunce::canonicalize(&source).expect("canonicalize source");
let root = dunce::canonicalize(dir.path()).expect("canonicalize root");

let manifest = RunnerManifest {
runtime_context: RuntimeContext {
Expand Down Expand Up @@ -3201,8 +3195,8 @@ mod tests {
let dir = tempfile::tempdir().expect("tempdir");
let source = dir.path().join("tool.py");
std::fs::write(&source, "VALUE = 1\n").expect("write source file");
let source = source.canonicalize().expect("canonicalize source");
let root = dir.path().canonicalize().expect("canonicalize root");
let source = dunce::canonicalize(&source).expect("canonicalize source");
let root = dunce::canonicalize(dir.path()).expect("canonicalize root");

let manifest = RunnerManifest {
runtime_context: RuntimeContext {
Expand Down Expand Up @@ -3246,8 +3240,8 @@ mod tests {
let dir = tempfile::tempdir().expect("tempdir");
let source = dir.path().join("tool.py");
std::fs::write(&source, "VALUE = 1\n").expect("write source file");
let source = source.canonicalize().expect("canonicalize source");
let root = dir.path().canonicalize().expect("canonicalize root");
let source = dunce::canonicalize(&source).expect("canonicalize source");
let root = dunce::canonicalize(dir.path()).expect("canonicalize root");

let manifest = RunnerManifest {
runtime_context: RuntimeContext {
Expand Down Expand Up @@ -3292,8 +3286,8 @@ mod tests {
let dir = tempfile::tempdir().expect("tempdir");
let source = dir.path().join("tool.py");
std::fs::write(&source, "VALUE = 1\n").expect("write source file");
let source = source.canonicalize().expect("canonicalize source");
let root = dir.path().canonicalize().expect("canonicalize root");
let source = dunce::canonicalize(&source).expect("canonicalize source");
let root = dunce::canonicalize(dir.path()).expect("canonicalize root");

let manifest = RunnerManifest {
runtime_context: RuntimeContext {
Expand Down Expand Up @@ -3341,8 +3335,8 @@ mod tests {
let dir = tempfile::tempdir().expect("tempdir");
let source = dir.path().join(filename);
std::fs::write(&source, "VALUE = 1\n").expect("write source file");
let source = source.canonicalize().expect("canonicalize source");
let root = dir.path().canonicalize().expect("canonicalize root");
let source = dunce::canonicalize(&source).expect("canonicalize source");
let root = dunce::canonicalize(dir.path()).expect("canonicalize root");

let manifest = RunnerManifest {
runtime_context: RuntimeContext {
Expand Down Expand Up @@ -3545,8 +3539,8 @@ mod tests {
let mut python = BTreeSet::new();
collect_from_dir(&root, &mut js_like, &mut python).expect("collect sources");

let inside = inside.canonicalize().expect("canonicalize inside");
let outside = outside.canonicalize().expect("canonicalize outside");
let inside = dunce::canonicalize(&inside).expect("canonicalize inside");
let outside = dunce::canonicalize(&outside).expect("canonicalize outside");
assert!(js_like.contains(&inside));
assert!(
!js_like.contains(&outside),
Expand Down Expand Up @@ -3585,6 +3579,37 @@ mod tests {
);
}

#[test]
fn collect_classified_files_yields_non_verbatim_paths() {
// On Windows `Path::canonicalize` emits `\\?\` verbatim paths the Python
// runner can't reconcile against a clean cwd; `dunce` must strip them.
let dir = tempfile::tempdir().expect("tempdir");
let source = dir.path().join("scorer.py");
std::fs::write(&source, "x = 1\n").expect("write source");

let classified =
collect_classified_files(std::slice::from_ref(&source)).expect("collect classified");
assert_eq!(classified.python.len(), 1, "expected the python source");

let collected = &classified.python[0];
assert!(collected.is_file(), "collected path must resolve to a file");
assert!(
!collected.to_string_lossy().starts_with(r"\\?\"),
"collected path must not carry a verbatim prefix, got {}",
collected.display()
);

// Allowed roots must be verbatim-free too, so the runner's clean
// cwd-based comparisons keep matching.
for root in &classified.allowed_roots {
assert!(
!root.to_string_lossy().starts_with(r"\\?\"),
"allowed root must not carry a verbatim prefix, got {}",
root.display()
);
}
}

fn test_base_args() -> BaseArgs {
BaseArgs {
json: false,
Expand Down
Loading