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

Vendir sync with http on windows fails because of a missing close before rename #359

Closed
meier-christoph opened this issue Feb 9, 2024 · 2 comments · Fixed by #360
Closed
Labels
bug This issue describes a defect or unexpected behavior carvel-accepted This issue should be considered for future work and that the triage process has been completed

Comments

@meier-christoph
Copy link
Contributor

What steps did you take:

git clone git@github.com:carvel-dev/vendir.git
cd vendir
go run .\cmd\vendir\vendir.go sync --chdir .\examples\http\

What happened:

The example fails on windows (in powershell).

Fetching: vendor + k8s-simple-app-plain (http from https://dk-shared-assets.s3.amazonaws.com/k8s-simple-app-example-master.zip)
vendir: Error: Syncing directory 'vendor':
  Syncing directory 'k8s-simple-app-plain' with HTTP contents: rename .vendir-tmp-632954061\incoming\vendir-http2106031697 .vendir-tmp-632954061\incoming\k8s-simple-app-example-master.zip:
    The process cannot access the file because it is being used by another process.
exit status 1

What did you expect:

It should download the files.

Anything else you would like to add:

I have the same problem with the latest version of vendir but you can reproduce it using the examples and the current develop branch.

The cause is a missing call to tmpFile.Close() before an os.Rename(tmpFile.Name(), archivePath) that is located here :
https://github.com/carvel-dev/vendir/blob/develop/pkg/vendir/fetch/http/sync.go#L51

You can fix it by closing the file first like this :

archivePath := filepath.Join(incomingTmpPath, path.Base(t.opts.URL))
tmpFile.Close() // add this line
err = os.Rename(tmpFile.Name(), archivePath)

Environment:

  • vendir version (execute vendir --version): current develop branch & latest release
  • OS (e.g. from /etc/os-release): windows

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@meier-christoph meier-christoph added bug This issue describes a defect or unexpected behavior carvel-triage This issue has not yet been reviewed for validity labels Feb 9, 2024
@joaopapereira
Copy link
Member

Thanks for reporting the issue and diagnosing the problem. Would you like to try it and create a PR with this fix?

@joaopapereira joaopapereira added carvel-accepted This issue should be considered for future work and that the triage process has been completed and removed carvel-triage This issue has not yet been reviewed for validity labels Feb 9, 2024
@meier-christoph
Copy link
Contributor Author

sure no problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior carvel-accepted This issue should be considered for future work and that the triage process has been completed
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants