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

Bug(v0.9.0-rc): Downloading modules fails with /tmp on tmpfs #370

Closed
ngergs opened this issue Jun 28, 2024 · 1 comment · Fixed by #375
Closed

Bug(v0.9.0-rc): Downloading modules fails with /tmp on tmpfs #370

ngergs opened this issue Jun 28, 2024 · 1 comment · Fixed by #375
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@ngergs
Copy link

ngergs commented Jun 28, 2024

Bug Report

1. Minimal reproduce step (Required)

Use the kcl cli and have on linux /tmp on a tmpfs (or any filesystem separate from where the cli will be executed)

kcl mod init && kcl mod add k8s

Relevant excerpt from mount:

tmpfs on /tmp type tmpfs (rw,nosuid,nodev,nr_inodes=1048576,inode64)

2. What did you expect to see? (Required)

No error ;)

3. What did you see instead (Required)

adding dependency 'k8s'
downloading 'kcl-lang/k8s:1.30' from 'ghcr.io/kcl-lang/k8s:1.30'
rename /tmp/336415861 /home/ngergs/.kcl/kpm/k8s_1.30: invalid cross-device link

4. What is your KCL components version? (Required)

kcl cli is 0.9.0-rc.1-linux-amd64

5. Error origin

The bug originates from here.

os.Rename is just a thin wrapper around syscall.Rename which errors if source and destination are not on the same filesystem.

Possible quickfixes that I see after glancing over it:

  • Initialize the tmp directory next to the destination so that they live on the same filesystem. This however could mean that orphaned files remain in edge case crash scenarios as there is no os cleanup mechanism like for /tmp present.
  • Handle the rename error via a errors.Is(syscall.EXDEV) check and fallback to copying. This of course looses the integrity guarantees renaming brings for systems where /tmp is not on the same filesystem.
@Peefy
Copy link
Contributor

Peefy commented Jun 29, 2024

Thanks for the feedback. PRs welcome or I'll take it later. ❤

@Peefy Peefy added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed labels Jun 29, 2024
@Peefy Peefy closed this as completed in #375 Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants