mirror of
https://github.com/storytold/filmcraft.git
synced 2026-10-24 10:10:05 -06:00
fix(engine): resolve stored media paths with craft-docpath's resolve_absolute (#2609)
* fix(engine): use resolve_absolute for stored media paths * fix(engine): give the reason when a scratch disk folder is refused * CI: check that git dependencies come from storytold * CI: release only with git dependencies from storytold * doc_path: a foreign drive path without a document is refused, not document-relative * Workflows: one Git-source check: lockfile present and current, no source replacement, storytold hosts only * Workflows: the Git-source check reads the legacy .cargo/config too * Workflows: the Git-source check parses cargo config with tomllib
This commit is contained in:
@@ -18,6 +18,44 @@ concurrency:
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
git-sources:
|
||||
name: Git dependencies come from storytold
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
steps:
|
||||
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
- name: Git dependencies come from storytold
|
||||
run: |
|
||||
set -euo pipefail
|
||||
test -f Cargo.lock || { echo "Cargo.lock is missing"; exit 1; }
|
||||
# Source replacement, patches and path overrides in cargo config never show up
|
||||
# in Cargo.lock. Parsed rather than grepped: TOML has many spellings of one key.
|
||||
# Cargo still reads the legacy .cargo/config.
|
||||
python3 - <<'PY'
|
||||
import os, sys, tomllib
|
||||
for path in (".cargo/config", ".cargo/config.toml"):
|
||||
if not os.path.exists(path):
|
||||
continue
|
||||
config = tomllib.load(open(path, "rb"))
|
||||
replaced = [name for name, source in config.get("source", {}).items()
|
||||
if isinstance(source, dict) and "replace-with" in source]
|
||||
if replaced or "patch" in config or "paths" in config:
|
||||
print(f"{path} replaces, patches or overrides a source")
|
||||
sys.exit(1)
|
||||
PY
|
||||
# Fails when Cargo.lock is out of date with the manifests.
|
||||
cargo metadata --locked --format-version 1 > /dev/null
|
||||
python3 - <<'PY'
|
||||
import sys, tomllib
|
||||
lock = tomllib.load(open("Cargo.lock", "rb"))
|
||||
bad = sorted({p["source"] for p in lock.get("package", [])
|
||||
if p.get("source", "").startswith("git+")
|
||||
and not p["source"].startswith("git+https://github.com/storytold/")})
|
||||
if bad:
|
||||
print("Git dependencies outside github.com/storytold:", *bad, sep="\n")
|
||||
sys.exit(1)
|
||||
PY
|
||||
|
||||
nix:
|
||||
# Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos.
|
||||
uses: storytold/craft-common-actions/.github/workflows/nix.yml@main
|
||||
|
||||
@@ -18,7 +18,47 @@ concurrency:
|
||||
cancel-in-progress: false
|
||||
|
||||
jobs:
|
||||
git-sources:
|
||||
name: Git dependencies come from storytold
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
contents: read
|
||||
steps:
|
||||
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
- name: Git dependencies come from storytold
|
||||
run: |
|
||||
set -euo pipefail
|
||||
test -f Cargo.lock || { echo "Cargo.lock is missing"; exit 1; }
|
||||
# Source replacement, patches and path overrides in cargo config never show up
|
||||
# in Cargo.lock. Parsed rather than grepped: TOML has many spellings of one key.
|
||||
# Cargo still reads the legacy .cargo/config.
|
||||
python3 - <<'PY'
|
||||
import os, sys, tomllib
|
||||
for path in (".cargo/config", ".cargo/config.toml"):
|
||||
if not os.path.exists(path):
|
||||
continue
|
||||
config = tomllib.load(open(path, "rb"))
|
||||
replaced = [name for name, source in config.get("source", {}).items()
|
||||
if isinstance(source, dict) and "replace-with" in source]
|
||||
if replaced or "patch" in config or "paths" in config:
|
||||
print(f"{path} replaces, patches or overrides a source")
|
||||
sys.exit(1)
|
||||
PY
|
||||
# Fails when Cargo.lock is out of date with the manifests.
|
||||
cargo metadata --locked --format-version 1 > /dev/null
|
||||
python3 - <<'PY'
|
||||
import sys, tomllib
|
||||
lock = tomllib.load(open("Cargo.lock", "rb"))
|
||||
bad = sorted({p["source"] for p in lock.get("package", [])
|
||||
if p.get("source", "").startswith("git+")
|
||||
and not p["source"].startswith("git+https://github.com/storytold/")})
|
||||
if bad:
|
||||
print("Git dependencies outside github.com/storytold:", *bad, sep="\n")
|
||||
sys.exit(1)
|
||||
PY
|
||||
|
||||
release:
|
||||
needs: git-sources
|
||||
# Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos.
|
||||
uses: storytold/craft-common-actions/.github/workflows/release.yml@main
|
||||
with:
|
||||
|
||||
@@ -39,15 +39,27 @@ impl std::error::Error for NotFollowed {}
|
||||
/// where it was opened from.
|
||||
pub fn resolve(stored: &str, document: Option<&Path>) -> Result<String, NotFollowed> {
|
||||
let document = document.filter(|d| !d.as_os_str().is_empty()).map(|d| std::path::absolute(d).unwrap_or_else(|_| d.to_path_buf()));
|
||||
let path: PathBuf = craft_docpath::resolve(stored, document.as_deref()).map_err(NotFollowed::Refused)?;
|
||||
// A path that is still relative would be taken from the working directory, which the
|
||||
// document does not choose.
|
||||
if path.is_relative() {
|
||||
return Err(NotFollowed::NoDocumentFolder);
|
||||
}
|
||||
let path: PathBuf = match craft_docpath::resolve_absolute(stored, document.as_deref()) {
|
||||
Ok(path) => path,
|
||||
// Without a document, a relative path resolves but stays relative.
|
||||
Err(Refused::Invalid) if document.is_none() && craft_docpath::resolve(stored, None).is_ok_and(|p| !foreign_root(&p)) => {
|
||||
return Err(NotFollowed::NoDocumentFolder);
|
||||
}
|
||||
Err(r) => return Err(NotFollowed::Refused(r)),
|
||||
};
|
||||
path.into_os_string().into_string().map_err(|_| NotFollowed::Refused(Refused::Invalid))
|
||||
}
|
||||
|
||||
/// Whether an unresolved `craft_docpath::resolve` result starts with a drive (`C:`) or a share
|
||||
/// root (`\\`) outside Windows. Such a path names no file here, and no document folder makes it one.
|
||||
fn foreign_root(path: &Path) -> bool {
|
||||
if cfg!(windows) {
|
||||
return false;
|
||||
}
|
||||
let b = path.as_os_str().as_encoded_bytes();
|
||||
b.starts_with(br"\\") || matches!((b.first(), b.get(1)), (Some(l), Some(b':')) if l.is_ascii_alphabetic())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -60,5 +72,27 @@ mod tests {
|
||||
assert_eq!(resolve("clip.mp4", None), Err(NotFollowed::NoDocumentFolder));
|
||||
assert_eq!(resolve(r"\\192.0.2.1\share\x.mp4", Some(&doc)), Err(NotFollowed::Refused(Refused::Network)));
|
||||
assert_eq!(resolve("file://192.0.2.1/s/x.mp4", Some(&doc)), Err(NotFollowed::Refused(Refused::Network)));
|
||||
assert_eq!(resolve("jar:take.mp4", Some(&doc)), Err(NotFollowed::Refused(Refused::Url)));
|
||||
// Colons in names: an alternate data stream or a URL on Windows, a plain character elsewhere.
|
||||
if cfg!(windows) {
|
||||
assert_eq!(resolve("media/take:2.mp4", Some(&doc)), Err(NotFollowed::Refused(Refused::Invalid)));
|
||||
assert_eq!(resolve("take:2.mp4", Some(&doc)), Err(NotFollowed::Refused(Refused::Url)));
|
||||
} else {
|
||||
assert_eq!(resolve("media/take:2.mp4", Some(&doc)), Ok(dir.join("media").join("take:2.mp4").to_string_lossy().into_owned()));
|
||||
assert_eq!(resolve("take:2.mp4", Some(&doc)), Ok(dir.join("take:2.mp4").to_string_lossy().into_owned()));
|
||||
}
|
||||
}
|
||||
|
||||
/// Without a document a relative path waits for the document's folder; outside Windows a
|
||||
/// drive path names no file and is refused.
|
||||
#[test]
|
||||
fn foreign_drive_paths_without_a_document_are_refused() {
|
||||
for stored in ["clip.mp4", "../clip.mp4"] {
|
||||
assert_eq!(resolve(stored, None), Err(NotFollowed::NoDocumentFolder), "{stored}");
|
||||
}
|
||||
#[cfg(unix)]
|
||||
for stored in [r"C:\x", r"C:x", "file:///C:/x/clip.mp4"] {
|
||||
assert_eq!(resolve(stored, None), Err(NotFollowed::Refused(Refused::Invalid)), "{stored}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1076,12 +1076,10 @@ fn project_settings_scratch(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
None => {}
|
||||
Some(Value::Null) => *slot = None,
|
||||
Some(Value::String(x)) if x.is_empty() || x.eq_ignore_ascii_case("same as project") => *slot = None,
|
||||
Some(Value::String(x))
|
||||
if matches!(crate::doc_path::resolve(x, s.path.as_deref().map(std::path::Path::new)), Err(crate::doc_path::NotFollowed::Refused(_))) =>
|
||||
{
|
||||
return Err(bad("file.projectSettings.scratchDisks", format!("`{k}` must be a folder on this computer, not a network share or device")));
|
||||
}
|
||||
Some(Value::String(x)) => {
|
||||
if let Err(crate::doc_path::NotFollowed::Refused(why)) = crate::doc_path::resolve(x, s.path.as_deref().map(std::path::Path::new)) {
|
||||
return Err(bad("file.projectSettings.scratchDisks", format!("`{k}` must be a folder on this computer: {why}")));
|
||||
}
|
||||
chosen.push(x.clone());
|
||||
*slot = Some(x.clone());
|
||||
}
|
||||
|
||||
@@ -307,6 +307,11 @@ fn media_properties_settings_and_scratch_disks() {
|
||||
assert!(s.execute("file.projectSettings.general", json!({"renderer": "quantum"})).is_err());
|
||||
s.execute("edit.undo", json!({})).unwrap();
|
||||
assert_eq!(s.project.settings.renderer, crate::project_tools::RENDERER_GPU);
|
||||
// Scratch Disks: a drive another system names is refused before the project has a folder
|
||||
if cfg!(unix) {
|
||||
assert!(s.execute("file.projectSettings.scratchDisks", json!({"captured": r"D:\Captured"})).is_err());
|
||||
assert!(s.project.settings.scratch.captured.is_none());
|
||||
}
|
||||
// Scratch Disks: previews follow the Video Previews folder once the project is saved
|
||||
let proj = dir.join("Show.fcproj").to_string_lossy().to_string();
|
||||
s.execute("file.saveAs", json!({"path": proj})).unwrap();
|
||||
|
||||
Reference in New Issue
Block a user