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

Jump to bottom

Fix updating credentials on different mounts/drives/volumes #1334

Merged
merged 5 commits into from
Jun 5, 2024
Merged

Fix updating credentials on different mounts/drives/volumes #1334

merged 5 commits into from
Jun 5, 2024

Conversation

quinchs
Copy link
Contributor

@quinchs quinchs commented May 23, 2024

Summary

This PR fixes #1330 by ensuring the temp file created for database in the project config is created in the same mount/drive/volume as its original file, permitting a rename.

@quinchs quinchs requested a review from aljazerzen May 23, 2024 11:49
scotttrinh
scotttrinh approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@scotttrinh scotttrinh left a comment

Choose a reason for hiding this comment

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

Changes look good to me!

src/branch/context.rs Show resolved Hide resolved
@aljazerzen
Copy link
Contributor

This does not fix the linked issue.

Here is some debugging info:

--- src/branch/context.rs:127 ---
                dbg!(&stash_path);
                let tmp = tmp_file_name(&stash_path);
                dbg!(&tmp);
                fs::write(&tmp, branch)?;
                fs::rename(&tmp, &stash_path.join("database"))?;

➜  edgedb-project2 pwd
/run/media/aljaz/USB/edgedb-project2
➜  edgedb-project2 edgedb branch switch x
Switching from 'main' to 'x'
[src/branch/context.rs:127:17] &stash_path = "/home/aljaz/.config/edgedb/projects/edged
b-project2-65ffeee7269f9567c19e74dd40460d99624093a3"
[src/branch/context.rs:129:17] &tmp = ".~edgedb-project2-65ffeee7269f9567c19e74dd40460d
99624093a3.tmp"
edgedb error: Invalid cross-device link (os error 18)

@aljazerzen
Copy link
Contributor

Ideally, we'd have a test for this somewhere (probably in docker) and host the project on a tmpfs.

@aljazerzen
Copy link
Contributor

I can confirm the bug is now fixed.

@quinchs quinchs merged commit f143df5 into master Jun 5, 2024
17 checks passed
@quinchs quinchs deleted the fix-branch-switch-cloud-path-for-credentials branch June 5, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scotttrinh scotttrinh scotttrinh approved these changes

@aljazerzen aljazerzen Awaiting requested review from aljazerzen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@quinchs @aljazerzen @scotttrinh