mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-24 10:09:32 -06:00
Footage, Lottie and models: resolve stored paths with craft-docpath's resolve_absolute (#1975)
* Footage: use resolve_absolute for stored paths * Lottie and models: use resolve_absolute for linked files * CI: release only with git dependencies from storytold * Use the shared git-sources check and pin checkout in release.yml * Resolve a model's side files through resolve_absolute; report C:\x as invalid * CI: parse cargo config, including the legacy .cargo/config, in the git-sources check
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -46,13 +46,24 @@ pub fn resolve(stored: &str, project: Option<&str>) -> Result<PathBuf, NotFollow
|
||||
let p = Path::new(p);
|
||||
std::path::absolute(p).unwrap_or_else(|_| p.to_path_buf())
|
||||
});
|
||||
let path = craft_docpath::resolve(stored, project.as_deref()).map_err(NotFollowed::Refused)?;
|
||||
// A path that is still relative would be taken from the working directory, which the
|
||||
// project does not choose.
|
||||
if path.is_relative() {
|
||||
return Err(NotFollowed::NoProjectFolder);
|
||||
}
|
||||
Ok(path)
|
||||
craft_docpath::resolve_absolute(stored, project.as_deref()).map_err(|why| match why {
|
||||
// Resolved, but still relative: it would be taken from the working directory, which the
|
||||
// project does not choose. A Windows drive or share path (`C:\x`) stays invalid outside
|
||||
// Windows even with a project.
|
||||
Refused::Invalid
|
||||
if project.is_none() && craft_docpath::resolve(stored, None).is_ok_and(|p| cfg!(windows) || !is_windows_rooted(&p)) =>
|
||||
{
|
||||
NotFollowed::NoProjectFolder
|
||||
}
|
||||
why => NotFollowed::Refused(why),
|
||||
})
|
||||
}
|
||||
|
||||
/// Whether `p` is a Windows drive or share path, which a project cannot make absolute outside Windows.
|
||||
fn is_windows_rooted(p: &Path) -> bool {
|
||||
let s = p.to_string_lossy();
|
||||
let b = s.as_bytes();
|
||||
(b.len() >= 2 && b[0].is_ascii_alphabetic() && b[1] == b':') || s.starts_with(r"\\") || s.starts_with("//")
|
||||
}
|
||||
|
||||
/// The paths of `f` (its file and every frame of an image sequence) resolved against the document
|
||||
|
||||
@@ -149,7 +149,23 @@ fn stored_paths_resolve_against_the_project() {
|
||||
let doc = doc.to_str().unwrap();
|
||||
assert_eq!(super::resolve("clip.mov", Some(doc)), Ok(std::env::temp_dir().join("art").join("clip.mov")));
|
||||
assert_eq!(super::resolve("clip.mov", None), Err(super::NotFollowed::NoProjectFolder));
|
||||
assert_eq!(super::resolve("../logo.png", None), Err(super::NotFollowed::NoProjectFolder));
|
||||
// A drive path is not a local file on Unix, with or without a project: it is not a missing project.
|
||||
#[cfg(unix)]
|
||||
for stored in [r"C:\x.mov", "C:x.mov"] {
|
||||
assert_eq!(super::resolve(stored, None), Err(super::NotFollowed::Refused(super::Refused::Invalid)), "{stored}");
|
||||
}
|
||||
assert_eq!(super::resolve(r"\\192.0.2.1\share\x.mov", Some(doc)), Err(super::NotFollowed::Refused(super::Refused::Network)));
|
||||
assert_eq!(super::resolve("jar:clip.mov", Some(doc)), Err(super::NotFollowed::Refused(super::Refused::Url)));
|
||||
// Windows reads a colon in a name as an alternate data stream; elsewhere it is a plain character.
|
||||
if cfg!(windows) {
|
||||
assert_eq!(super::resolve("shots/take:1.mov", Some(doc)), Err(super::NotFollowed::Refused(super::Refused::Invalid)));
|
||||
} else {
|
||||
assert_eq!(super::resolve("shots/take:1.mov", Some(doc)), Ok(std::env::temp_dir().join("art").join("shots").join("take:1.mov")));
|
||||
}
|
||||
// A Windows drive path names no file on other platforms.
|
||||
#[cfg(unix)]
|
||||
assert_eq!(super::resolve(r"C:\art\clip.mov", Some(doc)), Err(super::NotFollowed::Refused(super::Refused::Invalid)));
|
||||
#[cfg(windows)]
|
||||
assert!(super::resolve(r"\\server\share\x.mov", Some(r"\\SERVER\share\job\intro.ecproj")).is_ok());
|
||||
}
|
||||
|
||||
@@ -96,9 +96,11 @@ fn external_image(stored: &str, base_dir: &str) -> Result<String, String> {
|
||||
let doc = std::path::Path::new(base_dir).join("lottie.json");
|
||||
std::path::absolute(&doc).unwrap_or(doc)
|
||||
});
|
||||
match craft_docpath::resolve(stored, doc.as_deref()) {
|
||||
Ok(path) if path.is_absolute() => Ok(path.to_string_lossy().into_owned()),
|
||||
Ok(_) => Err(format!("{stored} is relative and the Lottie file's folder is not known")),
|
||||
match craft_docpath::resolve_absolute(stored, doc.as_deref()) {
|
||||
Ok(path) => Ok(path.to_string_lossy().into_owned()),
|
||||
Err(craft_docpath::Refused::Invalid) if doc.is_none() && craft_docpath::resolve(stored, None).is_ok() => {
|
||||
Err(format!("{stored} is relative and the Lottie file's folder is not known"))
|
||||
}
|
||||
Err(why) => Err(format!("{stored}: {why}")),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -559,7 +559,7 @@ impl Inner {
|
||||
let loaded = self.read(path).map_err(|e| e.to_string()).and_then(|bytes| {
|
||||
// Files the model names are read only when local or on the model's own network share.
|
||||
let resolve = |uri: &str| -> Option<Vec<u8>> {
|
||||
let p = craft_docpath::resolve(uri, Some(std::path::Path::new(path))).ok().filter(|p| p.is_absolute())?;
|
||||
let p = craft_docpath::resolve_absolute(uri, Some(std::path::Path::new(path))).ok()?;
|
||||
self.read(&p.to_string_lossy()).ok().map(|b| b.to_vec())
|
||||
};
|
||||
effectcraft_model::load(path, &bytes, &resolve).map_err(|e| e.to_string())
|
||||
|
||||
@@ -27,12 +27,21 @@ pub fn probe(path: impl AsRef<Path>) -> Result<Footage> {
|
||||
}
|
||||
let bytes = std::fs::read(path).map_err(|e| MediaError::Io(format!("{}: {e}", path.display())))?;
|
||||
if crate::MODEL_EXTENSIONS.contains(&ext_of(path).as_str()) {
|
||||
let dir = path.parent().map(Path::to_path_buf).unwrap_or_default();
|
||||
return probe_model(&path.to_string_lossy(), &bytes, &|uri| std::fs::read(dir.join(uri)).ok());
|
||||
return probe_model(&path.to_string_lossy(), &bytes, &side_file_reader(path));
|
||||
}
|
||||
probe_bytes(&path.to_string_lossy(), bytes.into())
|
||||
}
|
||||
|
||||
/// Reads the files a model file names (buffers, textures, MTL files). Like the pool, it follows only
|
||||
/// local files and files on the model's own network share: a URI is a path stored in the model.
|
||||
fn side_file_reader(model: &Path) -> impl Fn(&str) -> Option<Vec<u8>> + use<> {
|
||||
let model = std::path::absolute(model).unwrap_or_else(|_| model.to_path_buf());
|
||||
move |uri| {
|
||||
let p = craft_docpath::resolve_absolute(uri, Some(&model)).ok()?;
|
||||
std::fs::read(p).ok()
|
||||
}
|
||||
}
|
||||
|
||||
/// Probe a 3D model (glTF 2.0 / OBJ / STL / FBX): a [`FootageKind::Model`] item whose duration is its
|
||||
/// longest animation. `resolve` reads sibling resources.
|
||||
pub fn probe_model(path: &str, bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<Footage> {
|
||||
@@ -316,6 +325,20 @@ mod tests {
|
||||
(0..n).map(|k| (at(k + 1) - at(k)) as u32).collect()
|
||||
}
|
||||
|
||||
/// A model's side files are read from beside it; a URI to another computer is not followed.
|
||||
#[test]
|
||||
fn model_side_files_are_not_read_from_other_computers() {
|
||||
let dir = std::env::temp_dir().join(format!("ec-probe-side-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
std::fs::write(dir.join("buf.bin"), b"abc").unwrap();
|
||||
let read = side_file_reader(&dir.join("model.gltf"));
|
||||
assert_eq!(read("buf.bin"), Some(b"abc".to_vec()));
|
||||
assert_eq!(read(r"\\192.0.2.1\share\buf.bin"), None);
|
||||
assert_eq!(read("file://192.0.2.1/share/buf.bin"), None);
|
||||
assert_eq!(read(r"C:\buf.bin"), None);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn millisecond_rounded_frame_times_average_to_their_rate() {
|
||||
assert_eq!(rounded(FrameRate::FPS_30, 1000, 6), [33, 34, 33, 33, 34, 33]);
|
||||
|
||||
Reference in New Issue
Block a user