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

project render will fail if temp directory is in different device from render directory #10622

Closed
cscheid opened this issue Aug 27, 2024 · 4 comments · Fixed by #11095
Closed
Assignees
Labels
bug Something isn't working projects
Milestone

Comments

@cscheid
Copy link
Collaborator

cscheid commented Aug 27, 2024

directoryRelocator must also work across file systems:

const directoryRelocator = (destinationDir: string) => {
// move or copy dir
return (dir: string, copy = false) => {
const targetDir = join(destinationDir, dir);
const srcDir = join(projDir, dir);
// Dont' remove the directory unless there is a source
// directory that we can relocate
//
// If we intend for the directory relocated to be used to
// remove directories, we should instead make a function that
// does that explicitly, rather than as a side effect of a missing
// src Dir
if (existsSync(srcDir)) {
if (existsSync(targetDir)) {
Deno.removeSync(targetDir, { recursive: true });
}
ensureDirSync(dirname(targetDir));
if (copy) {
copyTo(srcDir, targetDir);
} else {
Deno.renameSync(srcDir, targetDir);
}
}
};
};

@cscheid cscheid added bug Something isn't working projects labels Aug 27, 2024
@cscheid cscheid added this to the v1.6 milestone Aug 27, 2024
@cscheid cscheid self-assigned this Aug 27, 2024
@cscheid
Copy link
Collaborator Author

cscheid commented Oct 17, 2024

I have a PR open, but I don't know how we could test this in CI...

@cderv
Copy link
Collaborator

cderv commented Oct 17, 2024

I think with Github action we have situation of different drive. Like on windows some Temp directory is on another drive (and I think I may have work around that because of Deno).

So we can probably make a test specific to github action that leverage this situation.

Unless this is not what different device is.

@cscheid
Copy link
Collaborator Author

cscheid commented Oct 17, 2024

Unless this is not what different device is.

You're right. But our test suite wasn't failing before this fix.

So we're not currently exercising it in CI (which is weird, because it's "just" a project render), or we are, and somehow this is still working.

@cderv
Copy link
Collaborator

cderv commented Oct 18, 2024

But our test suite wasn't failing before this fix.

I think this is because I worked around it at the time

- name: Fix temp dir to use runner one (windows)
if: runner.os == 'Windows'
run: |
echo "TMPDIR=${{ runner.temp }}" >> $GITHUB_ENV
echo "TMP=${{ runner.temp }}" >> $GITHUB_ENV
echo "TEMP=${{ runner.temp }}" >> $GITHUB_ENV
shell: bash

See af0ae78 - it was specifically for Deno 😓 so wrong way to handle as it has hidden the problem

It was a complement to something needed in R at the time, because it had some similar issues (960a061). Recent R may not have it.

We could remove - and then check that your PR solves it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working projects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants