Skip to content
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

resolve TempDir paths before passing them to the flake bin #233

Merged
merged 2 commits into from
Nov 8, 2024
Merged

resolve TempDir paths before passing them to the flake bin #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00fd486
resolve TempDir paths before passing them to the flake bin
valkum Oct 24, 2024
e5c3006
Revert ".github: Run workflows on push event only"
zhaofengli Nov 8, 2024
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 .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: Build
on:
pull_request:
push:
jobs:
build:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/linters.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
name: Linters

on:
pull_request:
push:
jobs:
linters:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: Tests
on:
pull_request:
push:
jobs:
tests:
Expand Down
5 changes: 4 additions & 1 deletion src/nix/hive/assets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ impl Assets {
// We explicitly specify `path:` instead of letting Nix resolve
// automatically, which would involve checking parent directories
// for a git repository.
let uri = format!("path:{}", temp_dir.path().to_str().unwrap());
let uri = format!(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a few usages of this: #236

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if those are relevant, but my thinking was that it might resolve other future regressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this near the point where the data (the temp path) leaves our control as this isn't a bug in the Rust side of things but at the boundary to a different system.
This only affects paths that we pass to nix (so ideally we would canonicalize the paths in FlakeMetadata::resolve). We could do that by passing something like a FlakePath to that function instead of the str.

enum FlakePath<'s> {
Path(&'s Path),
Git(&'s str)
}

<Display impl that canonicalizes before creating the `path:{}` string>

This would allow us to keep the simple temp dir paths in places where it could get debug printed without causing any irritation why the tmp dir looks weird. The symlinks for tempdirs on macos are an implementation detail and should not be exposed in level-1 debug output (such as a dbg!()) print, IMHO.

"path:{}",
temp_dir.path().canonicalize().unwrap().to_str().unwrap()
);
let _ = lock_flake_quiet(&uri).await;
let assets_flake = Flake::from_uri(uri).await?;
assets_flake_uri = Some(assets_flake.locked_uri().to_owned());
Expand Down