Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
disallowed-methods = [
{ path = "std::env::current_dir", reason = "The library must not depend on the process working directory: a call executed by the Lore service runs in a process whose directory is unrelated to the caller's. Use lore_revision::util::path::make_absolute, which resolves against the working directory the call carries." },
{ path = "dashmap::DashMap::entry", reason = "This function internally takes a write lock and returns it in the entry handle. This can potentially block worker threads and cause deadlocks if used incorrectly. If you're sure it's ok add #[allow(clippy::disallowed_methods)] with a comment explaining why its safe" },
{ path = "tokio::spawn", reason = "Use lore_base::lore_spawn! so LORE_CONTEXT is propagated to the spawned task." },
{ path = "tokio::task::spawn", reason = "Use lore_base::lore_spawn! so LORE_CONTEXT is propagated to the spawned task." },
Expand Down
6 changes: 6 additions & 0 deletions lore-capi/lore.h
Original file line number Diff line number Diff line change
Expand Up @@ -3530,6 +3530,12 @@ typedef struct lore_event_t {
typedef struct lore_global_args_t {
// Repository path
struct lore_string_t repository_path;
// Directory that relative paths in this call are resolved against. Set it
// when a call may be executed by another process, such as the Lore
// service, whose own working directory is unrelated to the caller's. When
// empty, relative paths resolve against the working directory of the
// process performing the call.
struct lore_string_t working_directory;
// Correlation ID
struct lore_string_t correlation_id;
// Identity to use
Expand Down
27 changes: 27 additions & 0 deletions lore-client/src/cli/commands/service/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,40 @@ pub enum ServiceMainError {}
/// left behind is a stale socket, which the next start detects and removes.
const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5);

/// Where the service parks its working directory. It inherits one from whoever
/// started it, which is unrelated to the directories its callers run in, and
/// holding that directory would also keep the filesystem under it busy. Callers
/// send the directory their relative paths belong to, so the service never
/// needs one of its own.
fn detached_working_directory() -> std::path::PathBuf {
#[cfg(target_family = "unix")]
{
std::path::PathBuf::from("/")
}
#[cfg(target_os = "windows")]
{
std::env::var_os("SystemRoot").map_or_else(
|| std::path::PathBuf::from("C:\\"),
std::path::PathBuf::from,
)
}
}

pub async fn service_main(
listening_signal: Option<tokio::sync::oneshot::Sender<()>>,
) -> Result<(), ServiceMainError> {
if !uds_supported() {
return Err(ServiceMainError::internal("IPC not supported on this OS"));
}

let detached = detached_working_directory();
if let Err(error) = std::env::set_current_dir(&detached) {
eprintln!(
"Failed to set working directory to {}: {error}",
detached.display()
);
}

let listener: UdsListener = UdsListener::new().internal("Failed to start listener socket")?;

if let Some(listening_signal) = listening_signal {
Expand Down
8 changes: 4 additions & 4 deletions lore-revision/src/file/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,14 @@ pub async fn write_file(
let destination = {
let mut absolute_path = Path::new(&output).to_path_buf();
if !absolute_path.is_absolute() {
let Ok(current_path) = std::env::current_dir() else {
let Ok(resolved) = crate::util::path::make_absolute(&output) else {
return Err(InvalidPath {
path: output.clone(),
}
.into());
};

absolute_path = current_path.join(output);
absolute_path = resolved;
}
absolute_path
};
Expand Down Expand Up @@ -295,14 +295,14 @@ pub async fn write_address(
let destination = {
let mut absolute_path = Path::new(&output).to_path_buf();
if !absolute_path.is_absolute() {
let Ok(current_path) = std::env::current_dir() else {
let Ok(resolved) = crate::util::path::make_absolute(&output) else {
return Err(InvalidPath {
path: output.clone(),
}
.into());
};

absolute_path = current_path.join(output);
absolute_path = resolved;
}
absolute_path
};
Expand Down
10 changes: 10 additions & 0 deletions lore-revision/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,12 @@ pub enum LoreLoadConfig {
pub struct LoreGlobalArgs {
/// Repository path
pub repository_path: LoreString,
/// Directory that relative paths in this call are resolved against. Set it
/// when a call may be executed by another process, such as the Lore
/// service, whose own working directory is unrelated to the caller's. When
/// empty, relative paths resolve against the working directory of the
/// process performing the call.
pub working_directory: LoreString,
/// Correlation ID
pub correlation_id: LoreString,
/// Identity to use
Expand Down Expand Up @@ -579,6 +585,10 @@ impl LoreGlobalArgs {
self.repository_path.as_str()
}

pub fn working_directory(&self) -> Option<&str> {
(&self.working_directory).into()
}

pub fn identity(&self) -> Option<&str> {
(&self.identity).into()
}
Expand Down
38 changes: 29 additions & 9 deletions lore-revision/src/util/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,42 @@ pub enum PathError {
InvalidPath,
}

/// Resolves `path` against the working directory of the call in progress, or
/// against this process's own when the call did not name one.
///
/// A call arriving over IPC carries the directory of the process that made it,
/// because the service's own is unrelated to the caller's.
pub fn make_absolute(path: impl AsRef<str>) -> Result<PathBuf, PathError> {
let context = crate::runtime::try_execution_context();
let base = context
.as_ref()
.and_then(|context| context.globals().working_directory())
.map(Path::new);
make_absolute_from(path, base)
}

/// [`make_absolute`] with the base directory supplied by the caller, for the
/// call wrappers that resolve paths before the execution context exists and so
/// cannot look it up.
pub fn make_absolute_from(
path: impl AsRef<str>,
base: Option<&Path>,
) -> Result<PathBuf, PathError> {
let path = path.as_ref();
let cleanpath = clean(path.to_owned());
let pathbuf = PathBuf::from_str(cleanpath.as_str()).emit_map_err(InvalidPath {
path: path.to_string(),
})?;
if !pathbuf.is_absolute() {
Ok(std::env::current_dir()
if pathbuf.is_absolute() {
return Ok(pathbuf);
}
match base {
Some(base) => Ok(base.join(pathbuf)),
None => Ok(std::env::current_dir()
.emit_map_err(PathError::internal(
"failed to get current working directory",
))?
.join(pathbuf))
} else {
Ok(pathbuf)
.join(pathbuf)),
}
}

Expand Down Expand Up @@ -651,15 +673,13 @@ impl RelativePathBuf {

let mut absolute_path = Path::new(user_path).to_path_buf();
if !absolute_path.is_absolute() {
absolute_path = std::path::absolute(absolute_path)
.emit_map_err(PathError::internal("failed to resolve absolute path"))?;
absolute_path = make_absolute(absolute_path.to_string_lossy())?;
}
let absolute_path = clean(absolute_path.display().to_string());

let mut repository_path = Path::new(repository_path).to_path_buf();
if !repository_path.is_absolute() {
repository_path = std::path::absolute(repository_path)
.emit_map_err(PathError::internal("failed to resolve absolute path"))?;
repository_path = make_absolute(repository_path.to_string_lossy())?;
}
let repository_path = clean(repository_path.display().to_string()).to_lowercase();

Expand Down
17 changes: 10 additions & 7 deletions lore/src/call.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// SPDX-FileCopyrightText: 2026 Epic Games, Inc.
// SPDX-License-Identifier: MIT
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Instant;
Expand Down Expand Up @@ -221,13 +222,15 @@ async fn prepare_repository_call(
mut globals: LoreGlobalArgs,
callback: LoreEventCallback,
) -> Result<(PathBuf, Arc<ExecutionContext>), i32> {
let repository_path =
if let Ok(path) = util::path::make_absolute(globals.repository_path.as_str()) {
globals.repository_path = path.display().to_string().into();
path
} else {
PathBuf::from(globals.repository_path.as_str())
};
let repository_path = if let Ok(path) = util::path::make_absolute_from(
globals.repository_path.as_str(),
globals.working_directory().map(Path::new),
) {
globals.repository_path = path.display().to_string().into();
path
} else {
PathBuf::from(globals.repository_path.as_str())
};

let execution = setup_execution(globals, callback);

Expand Down
19 changes: 18 additions & 1 deletion lore/src/remote/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,28 @@ impl EventError for ServiceCallError {
}
}

/// Records the directory the service resolves this call's relative paths
/// against, when the caller left it unset. The service runs in a directory
/// unrelated to the caller's, so without this a relative path would resolve
/// there rather than where the caller ran. A caller that set the field, such as
/// an installed tool that runs from a fixed directory but wants relative paths
/// resolved elsewhere, keeps its value.
#[allow(clippy::disallowed_methods)]
fn fill_working_directory(globals: &mut LoreGlobalArgs) {
if globals.working_directory().is_some() {
return;
}
if let Ok(directory) = std::env::current_dir() {
globals.working_directory = directory.display().to_string().into();
}
}

pub async fn service_call<ArgsType: LoreArgs + Clone + Send + 'static>(
globals: LoreGlobalArgs,
mut globals: LoreGlobalArgs,
args: ArgsType,
callback: LoreEventCallback,
) -> i32 {
fill_working_directory(&mut globals);
let mut event_dispatcher = EventDispatcher::new(callback);

service_call_impl(&mut event_dispatcher, globals, args)
Expand Down
29 changes: 29 additions & 0 deletions scripts/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,35 @@ def _wait_for_service_ready(lore_executable_path, service_process, attempts=30):
pytest.fail("Timed out waiting for Lore background service to accept connections")


@pytest.fixture(scope="function")
def lore_service_in_directory(lore_executable_path, global_dir_name):
"""Starts the service in a chosen directory, for tests that need the
service's own working directory to differ from the caller's. The service
shares the test's isolated global config so shared stores it creates land
where the client looks for them."""
processes = []

def start(directory):
env = os.environ.copy()
env["LORE_GLOBAL_PATH"] = global_dir_name
service_process = subprocess.Popen(
[lore_executable_path, "service", "run"], cwd=str(directory), env=env
)
processes.append(service_process)
_wait_for_service_ready(lore_executable_path, service_process)
return service_process

yield start

for service_process in processes:
service_process.terminate()
try:
service_process.wait(timeout=10)
except subprocess.TimeoutExpired:
logger.warning("Lore service did not exit on terminate, killing it")
service_process.kill()


@pytest.fixture(scope="function")
def background_lore_service(lore_executable_path):
command_args = [lore_executable_path, "service", "run"]
Expand Down
75 changes: 75 additions & 0 deletions scripts/test/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def service_supported():


@pytest.mark.smoke
@pytest.mark.xdist_group("lore_service")
@pytest.mark.skip(reason="Unknown issue specifically running in CI for OSS")
@pytest.mark.skipif(
not service_supported(), reason="Service not supported on " + platform.system()
Expand All @@ -29,6 +30,7 @@ def test_service_down(new_lore_repo):


@pytest.mark.smoke
@pytest.mark.xdist_group("lore_service")
@pytest.mark.skipif(
not service_supported(), reason="Service not supported on " + platform.system()
)
Expand All @@ -48,3 +50,76 @@ def test_service_call(new_lore_repo, background_lore_service):
assert "A " + file_name in map(
lambda line: line.strip(" "), status_output.splitlines()
)


@pytest.mark.smoke
@pytest.mark.xdist_group("lore_service")
@pytest.mark.skipif(
not service_supported(), reason="Service not supported on " + platform.system()
)
def test_service_resolves_relative_paths_against_caller(
new_lore_repo, lore_service_in_directory, tmp_path
):
"""Relative paths belong to the directory the command was run in.

The service resolves them, and its own working directory is unrelated to
the caller's, so a service started elsewhere must not pull them towards
itself. Every other service test passes an absolute repository path, which
cannot catch this.
"""
# Start the service in a directory unrelated to where the commands run, so
# that a relative path resolved there rather than at the caller would show.
service_directory = tmp_path / "service_elsewhere"
caller_directory = tmp_path / "caller"
service_directory.mkdir()
caller_directory.mkdir()
lore_service_in_directory(service_directory)

# Seed a remote to clone from. Routed through the service like the rest,
# but against the repository's own absolute path, so unaffected by the
# service's directory.
source: Lore = new_lore_repo(environment_vars=LORE_SERVICE_ENVIRONMENT.copy())
with source.open_file("seed.txt", "w+") as seed_file:
seed_file.write("seed\n")
source.stage(scan=True, offline=True)
source.commit("Seed", offline=True)
source.push()

# Clone to a relative path from the caller's directory. It must land there,
# not under the service's directory.
clone_name = "relative_clone"
source.run(
["repository", "clone", source.remote_path, clone_name],
cwd=str(caller_directory),
use_os_dir=True,
)

clone_path = caller_directory / clone_name
assert (clone_path / ".lore").is_dir(), (
f"Clone must land under the caller's directory, not the service's. "
f"{caller_directory} contains {list(caller_directory.iterdir())}"
)
assert not (service_directory / clone_name).exists(), (
f"Clone must not land under the service's directory. "
f"{service_directory} contains {list(service_directory.iterdir())}"
)

# Stage a relative path from inside the clone.
clone = Lore(
lore_executable_path=source.lore_executable_path,
path=str(clone_path),
name=clone_name,
global_dir=source.global_dir,
environment_vars=LORE_SERVICE_ENVIRONMENT.copy(),
remote_url=source.remote,
remote_path=source.remote_path,
create_repo=False,
)
file_name = "added.uasset"
(clone_path / file_name).write_bytes(os.urandom(30))
clone.stage(file_name, relative_paths=True)

status_output = clone.status()
assert "A " + file_name in map(
lambda line: line.strip(" "), status_output.splitlines()
), f"Staged file should show as added: {status_output}"