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.

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

rustpkg: Failing on rename between cross-device #9887

Closed
dpc opened this issue Oct 16, 2013 · 9 comments
Closed

rustpkg: Failing on rename between cross-device #9887

dpc opened this issue Oct 16, 2013 · 9 comments

Comments

@dpc
Copy link
Contributor

dpc commented Oct 16, 2013

I've found what could be the reason why rustpkg does not work on my system that I've reported in the comments to: http://tim.dreamwidth.org/1820526.html

[pid 25620] stat("/home/dpc/lab/rust/.rust/src/github.com/steveklabnik", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
[pid 25620] rename("/tmp/jYDYGA3OTuHfMQwLrustpkg/rustpkg_temp", "/home/dpc/lab/rust/.rust/src/github.com/steveklabnik/hello") = -1 EXDEV (Invalid cross-device link)
[pid 25620] write(2, "task <unnamed> failed at 'Unhand"..., 381task <unnamed> failed at 'Unhandled condition: nonexistent_package: (package_id::PkgId{path: std::path::PosixPath{is_absolute: false, components: ~[~"github.com", ~"steveklabnik", ~"hello"]}, short_name: ~"hello", version: NoVersion}, ~"supplied path for package dir does not exist, and couldn't interpret it as a URL fragment")', /home/dpc/opt/src/rust/src/libstd/condition.rs:131) = 381
@huonw
Copy link
Member

huonw commented Oct 16, 2013

cc @catamorphism, seems similar to/the same as #9781.

@dpc
Copy link
Contributor Author

dpc commented Oct 16, 2013

BTW. My attempt to fix the issue:

diff --git a/src/librustpkg/package_source.rs b/src/librustpkg/package_source.rs
index c2fddaf..f693a49 100644
--- a/src/librustpkg/package_source.rs
+++ b/src/librustpkg/package_source.rs
@@ -202,7 +202,7 @@ impl PkgSrc {
             // Since the operation succeeded, move clone_target to local.
             // First, create all ancestor directories.
             if make_dir_rwx_recursive(&local.pop())
-                && os::rename_file(&clone_target, local) {
+                && os::copy_file(&clone_target, local) {
                  Some(local.clone())
             }
             else {

Does not work neither. It seems I'm getting empty file (size == 0).

[mutex:lab/rust]% ls -alh /home/dpc/lab/rust/.rust/src/github.com/steveklabnik/hello-0.1
-rwxrwxr-x 1 dpc dpc 0 paź 15 22:46 /home/dpc/lab/rust/.rust/src/github.com/steveklabnik/hello-0.1

@dpc
Copy link
Contributor Author

dpc commented Oct 19, 2013

diff --git a/src/librustpkg/package_source.rs b/src/librustpkg/package_source.rs
index c2fddaf..00a764b 100644
--- a/src/librustpkg/package_source.rs
+++ b/src/librustpkg/package_source.rs
@@ -170,7 +170,7 @@ impl PkgSrc {
         // We use a temporary directory because if the git clone fails,
         // it creates the target directory anyway and doesn't delete it

-        let scratch_dir = extra::tempfile::mkdtemp(&os::tmpdir(), "rustpkg");
+        let scratch_dir = extra::tempfile::mkdtemp(&Path(&"/home/dpc/lab/rust/tmp"), "rustpkg");
         let clone_target = match scratch_dir {
             Some(d) => d.push("rustpkg_temp"),
             None    => cond.raise(~"Failed to create temporary directory for fetching git sources")

This works, so it confirms the problem is using rename to move files between filesystems. This will not work with every system that has /tmp mounted eg. as ramfs.

@catamorphism
Copy link
Contributor

Thanks for the update, @dpc -- another workaround is to set TMPDIR in the environment to something under your /home.

@dpc
Copy link
Contributor Author

dpc commented Oct 19, 2013

@catamorphism: Ah! Yes, I forgot about this.

ATM, I'm looking for why did copy_file fail. Am I right, that it should just work?

@dpc
Copy link
Contributor Author

dpc commented Oct 19, 2013

OK. I've figured it out. #9947 . Now that puzzle is solved, I might happily enjoy rustpkg command. :)

@derekchiang
Copy link
Contributor

I see two ways to resolve this issue:

  1. Use something like ./.rust/tmp as the temporary directory by default.
  2. When seeing this IO error, degrade to copying and deleting the source folder, instead of renaming.

Thoughts?

@derekchiang
Copy link
Contributor

Also, this actually is a libuv issue. The same error can be reproduced with Node, as described here.

@alexcrichton
Copy link
Member

Closing, rustpkg was removed.

flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 12, 2023
fix empty_structs_with_brackets suggestion errors

fixes rust-lang#9887

I refer to [my comment](rust-lang/rust-clippy#9887 (comment)) to explain this PR.

---

changelog: Sugg: [`empty_structs_with_brackets`]: The suggestion is no longer machine applicable, to avoid errors when accessing struct fields
[rust-lang#10141](rust-lang/rust-clippy#10141)
<!-- changelog_checked -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants